search

spring-io / pivotal-cla

Apache License 2.0
10 stars 16 forks source link

Make obvious fix trigger word a little more explicit #150

Closed simonbasle closed 5 years ago

simonbasle commented 7 years ago

In #135, a feature to indicate that a PR is considered as an obvious fiix was introduced.

However, the trigger is too generic and prone to false positives: the words obvious and fix could be used liberally in any PR / comment, and that would trigger the bypass. See reactor/reactor-netty#31 for an example of a situation where it occurred...

The trigger could be made more specific by requiring the words to be between square brackets in order to bypass the CLA.

@mp911de I also wondered if it was possible to remove the flag (and have the CLA re-evaluated) once the PR has been marked as obvious by the bot? Eg. do you know if editing the message to remove the trigger would work?

cc @rwinch

mp911de commented 7 years ago

Removing obvious fix from the description/title/comment causes the CLA tool to fall back to CLA required. We discussed also an approach by using labels but there's no standardization and we didn't want to enforce a label that does not fit the projects' way of working.

simonbasle commented 7 years ago

That does sound fair, so one concern less :) Switching from "obvious fix" to "[obvious fix]" as the trigger shouldn't be too much of a problem, I'll see if I can get a PR together soon!

mp911de commented 7 years ago

I skimmed Boot's PR's and found an pretty good example:

@pivotal-issuemaster This is an obvious fix

See https://github.com/spring-projects/spring-boot/pull/6991#issuecomment-248867379

Mentioning pivotal-issuemaster in the same field adds a pretty good context to the message and I think that's better than adding brackets, quotes, etc.

We need to tell all users about the change once it's in place.

@rwinch thoughts?

simonbasle commented 5 years ago

Note that the fix in 0d36b11 is a slightly hybrid approach: in order to let users trigger the obvious fix anywhere in comments and in the PR body, the trigger is now more precise and includes the mention of the bot: @bot this is an obvious fix.