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

Remove small patch exception checking in hack/make/validate-dco #9083

Conversation

SvenDowideit
Copy link
Contributor

I propose allowing maintainers to add a DCO when its missing from the git commit.

Either by asking the commiter to add it to the Pull Request's comments, or by deciding its a trivial change, so can be signed by them.

replaces #9010
#9079 is an example of this in action.

@SvenDowideit SvenDowideit force-pushed the pr_out_remove_small_patch_exception_checking_in_hack_make_validate_dco branch 2 times, most recently from 3474083 to f66bf37 Compare November 11, 2014 06:18
@SvenDowideit
Copy link
Contributor Author

@fredlf @jamtur01 to expand on this, the workflow I used was:

  • ask for DCO in the PR
  • run pulls 9010
  • git commit --amend -s, add the commiters signature to the commit
  • pulls send

and then close the original PR, and add some nice words to the new one.

If this is doesn't break the DCO chain ( @keeb ), then we have a workable solution, which we can further optimize by adding some more code to pulls

@crosbymichael @tianon @shykes

@SvenDowideit
Copy link
Contributor Author

after discussing it a little, @tiborvass noted that we can automate the DCO in PR comment -> takeover PR and sign - I guess I'll be playing with some hooks soon :)

@keeb-zz
Copy link
Contributor

keeb-zz commented Nov 11, 2014

It only makes sense that, if a comment a contribution is trivial / falls under the trivial guidelines for the agreement, that someone could theoretically sign it and contribute if the contributor has abandoned the process. Automation is also a win!

I will check with legal to make sure that I am not misunderstanding.

@fredlf
Copy link
Contributor

fredlf commented Nov 11, 2014

I don't really see how any of this makes life simpler for docs maintainers or contributors, but that's clearly not the use case we are solving for, so I will accept whatever the core maintainers say is the best workflow for docs to use.

@crosbymichael
Copy link
Contributor

LGTM

I'll close my PR for now and we can move forward with this version after @keeb can verify the workflow.

@tianon
Copy link
Member

tianon commented Nov 13, 2014

LGTM pending @keeb's confirmation

@tiborvass
Copy link
Contributor

@SvenDowideit This will need a rebase

Ping @keeb for confirmation.

@crosbymichael crosbymichael added this to the 1.5.0 milestone Dec 16, 2014
crosbymichael and others added 3 commits December 17, 2014 10:02
As we move forward on automating our pull request review process and
tooling these exceptions hurt more than they help.  For consistency we
should not allow small patch exceptions for anything.  The source of
truth going forward for DCO and builds are the official drone status on
each pull request.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Signed-off-by: Andrew Page <admwiggin@gmail.com>
…ge, or to use their own if its a trvial change

Signed-off-by: Sven Dowideit <SvenDowideit@home.org.au>
@SvenDowideit SvenDowideit force-pushed the pr_out_remove_small_patch_exception_checking_in_hack_make_validate_dco branch from f66bf37 to 56e3f49 Compare December 17, 2014 00:03
@@ -4,7 +4,7 @@ source "$(dirname "$BASH_SOURCE")/.validate"

adds=$(validate_diff --numstat | awk '{ s += $1 } END { print s }')
dels=$(validate_diff --numstat | awk '{ s += $2 } END { print s }')
notDocs="$(validate_diff --numstat | awk '$3 !~ /^docs\// { print $3 }')"
#notDocs="$(validate_diff --numstat | awk '$3 !~ /^docs\// { print $3 }')"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn' t we remove this?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's fine for now as it's what tianon wanted

@crosbymichael
Copy link
Contributor

ping @jfrazelle

@jessfraz
Copy link
Contributor

LGTM

jessfraz pushed a commit that referenced this pull request Dec 17, 2014
…exception_checking_in_hack_make_validate_dco

Remove small patch exception checking in hack/make/validate-dco
@jessfraz jessfraz merged commit d06f496 into moby:master Dec 17, 2014
@jessfraz
Copy link
Contributor

lol @SvenDowideit at your branch name, do you think its long enough?

@SvenDowideit
Copy link
Contributor Author

@jfrazelle grin - blame gordon, that one what chosen by pulls send

@SvenDowideit SvenDowideit deleted the pr_out_remove_small_patch_exception_checking_in_hack_make_validate_dco branch April 26, 2015 23:32
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

8 participants