Skip to content

Enable PR linting with Danger#9

Open
harpojaeger wants to merge 11 commits intospokecommunity:masterfrom
harpojaeger:enable-danger-linting
Open

Enable PR linting with Danger#9
harpojaeger wants to merge 11 commits intospokecommunity:masterfrom
harpojaeger:enable-danger-linting

Conversation

@harpojaeger
Copy link

This PR enables automatic linting of the files that were changed in any given PR. On success, a message like this one is added by a bot:
Screen Shot 2019-03-20 at 20 21 01
On failure/warning, it looks like this:
Screen Shot 2019-03-20 at 20 21 10

After this is merged, some trivial Travis CI setup is necessary to complete the process (I'll handle that).

dangerfile.js is where the actual linting logic is performed (this can be used for lots of things besides linting). Updates to .travis.yml are necessary to actually run Danger as part of CI. It was necessary to make .eslintrc into a JSON file so it could be imported and parsed in the Dangerfile. Also, I disabled auto-fixing in the default npm run lint script.

@harpojaeger harpojaeger requested a review from a team March 21, 2019 00:34
Copy link
Member

@bchrobot bchrobot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small nits from me, otherwise it looks good 👍

const resultsTableHeader = '### Linter results\r|File|Errors|Warnings\r|---|---|---|\r'
const resultsTableRows = lintReport.results.map(file => {
const travisBasePath = /home\/travis\/build\/\w+\/Spoke\//
const localPath = file.filePath.replace(travisBasePath, '')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think moving this outside of map will provide better performance

const { modified_files: modified, created_files: created } = danger.git
const linter = new CLIEngine(config)
const lintReport = linter.executeOnFiles([...modified, ...created])
// console.log(lintReport.results)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove line

@jlegrone jlegrone changed the base branch from main to master April 14, 2019 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants