Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add config option to only lint commits #33

Merged
merged 1 commit into from
Dec 9, 2018

Conversation

stefanbuck
Copy link
Contributor

@stefanbuck stefanbuck commented Dec 5, 2018

Useful when using Rebase + Merge as merge strategy.

Closes #31


View rendered README.md

const commits = await context.github.pullRequests.getCommits(context.repo({
number: context.payload.pull_request.number
}))

return commits.data
.map(element => element.commit)
.some(commit => isSemanticMessage(commit.message))
.map(element => element.commit)[allCommits ? 'every' : 'some'](commit => isSemanticMessage(commit.message))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is operation to invoke either every or some on the array can be controversial. I'm happy to change it to a more verbose version if you like

Copy link
Owner

Choose a reason for hiding this comment

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

Clever!

const state = isSemantic ? 'success' : 'pending'

function getDescription () {
if (hasSemanticTitle) return 'ready to be squashed'
if (hasSemanticCommits) return 'ready to be merged or rebased'
if (titleOnly) return 'add a semantic PR title'
if (commitsOnly) return 'add a semantic commit'
Copy link
Owner

@zeke zeke Dec 6, 2018

Choose a reason for hiding this comment

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

add a semantic commit

Shouldn't this message be something more like "make sure every commit is semantic"?

This gets at the root of the trouble with this new commitsOnly option: People will have to amend their commit history in order to comply with it. Many people don't know how to do that. I guess the challenge here is to convey that in a very short status message...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Really good point. I've updated the wording as suggested.

Useful when using Rebase + Merge as merge strategy.

Closes zeke#31
@nathanielc
Copy link

I am new to using Github Apps, since I have configured the bot via the normal Github App workflow when can I expect changes in this repo to be live in the app? Does an official release have to be made first?

I ask because I am not sure if I have configured the bot correctly or not via this PR influxdata/flux#444.

@zeke
Copy link
Owner

zeke commented Dec 12, 2018

when can I expect changes in this repo to be live in the app?

This app is deployed to Heroku and integrated with GitHub to autodeploy changes to the master branch. So every PR should be live within a few minutes of merging.

I am not sure if I have configured the bot correctly

It makes sense to me that your test PR is passing: there's only one commit and it has a semantic prefix: influxdata/flux@20ab001

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.

None yet

3 participants