Bug 498340 - ip-validation fails when creating pull request through web editor
Summary: ip-validation fails when creating pull request through web editor
Status: RESOLVED FIXED
Alias: None
Product: Community
Classification: Eclipse Foundation
Component: GitHub (show other bugs)
Version: unspecified   Edit
Hardware: PC Mac OS X
: P3 normal with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Christopher Guindon CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 540694
Blocks:
  Show dependency tree
 
Reported: 2016-07-22 09:01 EDT by Karsten Thoms CLA
Modified: 2019-03-18 09:40 EDT (History)
6 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Karsten Thoms CLA 2016-07-22 09:01:12 EDT
I have edited a file through GitHub's web interface and created a pull request from my branch to the original project (eclipse/xtext). After creation of the pull request I get a validation problem for ip-validation.

- I have signed a CLA in Eclipse Portal
- My GitHub account name (kthoms) is registered in my Eclipse profile
- My bugzilla email address is registered as primary email address on GitHub
- My email address is public in GitHub setting

How do I sign off such a change through the GitHub web editor?

--------------------------------------------------------------------------------
Current status
The pull request did not pass Eclipse validation.

    The following users do not have valid CLA. Please see this page for details, or to sign a CLA.
        noreply@github.com

    The following users have invalid Signed-off-by footers. Please use the Signed-Off-By (git commit -s) mechanism to confirm the origin of the submission. The identity in the Signed-off-By must match the Author or the Committer.
        noreply@github.com
--------------------------------------------------------------------------------
Comment 1 Frederic Gurr CLA 2016-07-25 06:26:28 EDT
I'm not aware of a way to do this through the GitHub web editor. You could raise an issue at GitHub to find out if this is supported.
Comment 2 Karsten Thoms CLA 2016-07-25 06:59:46 EDT
Contacted GitHub support. Let's see what they say.
Comment 3 Karsten Thoms CLA 2016-07-29 10:58:48 EDT
Sorry for coming back. I had some email discussion with GitHub and they don't see a problem on their side:


--------------------------------------------------------------------------------

Am 29.07.2016 um 15:49 schrieb Ivan Žužak <support@github.com>:

Hi Karsten,

The behavior you observed is expected from GitHub's side. Here's what the metadata of the commit in that pull request looks like:

https://api.github.com/repos/eclipse/xtext/commits/f4f303f3a3a9ff5c95da956624762ae7cb355f00

   "author": {
     "name": "Karsten Thoms",
     "email": "karsten.thoms@itemis.de",
     "date": "2016-07-26T11:13:24Z"
   },
   "committer": {
     "name": "GitHub",
     "email": "noreply@github.com",
     "date": "2016-07-26T11:13:24Z"
   },


The committer is set to GitHub because you created the commit on GitHub. However, the Author is set to you because you authored the commit. Instead of using the committer's email address and information, you should be using the author's email address and information if you want to run validations on who authored the commit.

I hope that explains things, but let me know if you have any other questions.

Cheers,
Ivan
--------------------------------------------------------------------------------






Am 29.07.2016 um 16:50 schrieb Ivan Žužak <support@github.com>:

Hi Karsten,

>>kthoms>> Does "you should be using the author's email address and information“ mean „Eclipse should be using the author's email address and information“ and the ip-validation should use that? 

Sorry for not being clearer. It's hard for us to say what systems that we didn't build or maintain should be using. I'm just sharing information about how things work from our end and guessing what might be causing problems. Does that make sense? If the maintainers of the systems you're referring to have any questions about this for us, please let them know that they can reach out: https://github.com/contact

>>kthoms>> I have no influence on the ip-validation check myself, but do you think I should get back with that information to the Eclipse webmaster? 

You're free to make that choice for yourself -- we're okay with you sharing that information, we're okay with you not sharing that information, and we're okay with you just pointing them to our contact form if they have any questions.

>>kthoms>> Did you contacted them for clarification?

No.

>>kthoms>> I am „just“ a contributor who wants to use the GitHub web interface to edit minor text changes, but if you say that Eclipse’s ip-validation check is wrong here, I could contact them again (webmaster@eclipse.org <mailto:webmaster@eclipse.org>).

We can't say that because we didn't build that ip_validation system nor do we maintain it -- what that system does and how it does it is not up to us, but we're happy to provide information about how GitHub works so that the people who do maintain the system can make the choices they need. 

I hope that clarifies things.

Best,
Ivan
Comment 4 Michael Keppler CLA 2017-02-12 09:55:26 EST
I suggest to raise the priority of this issue. Github was chosen as hosting site besides the Eclipse own hosting to allow projects to use more flexibility and different ways to work with their communities.

One of the Github benefits is that "drive by contributions" are really easy, e.g. fixing a typo is a matter of clicking the edit button right on the webpage and after 20 seconds you are done (without cloning the repository etc.).

This bug kills those drive-by-contributions completely, because the IP validation reads the wrong mail field from the webhook. Now I have to clone entire project repositories to my local machine for simple typo fixes.
Comment 5 Christian Dietrich CLA 2017-02-27 10:02:50 EST
There is the same issue with the merge commits done via github web.
if i want to merge some commits incl merge commits from one branch to another the ip validation complains again
Comment 6 Karsten Thoms CLA 2017-04-24 10:58:53 EDT
Please reconsider this bug. It is preventing Eclipse collaborators using GitHub's web editor completely.
Comment 7 Denis Roy CLA 2017-05-03 16:18:12 EDT
Absolutely. I'm targeting a few GH bugs in the following weeks.
Comment 8 Karsten Thoms CLA 2017-05-04 02:36:21 EDT
Thanks, Denis. Would be awesome if you could solve this issue.
Comment 9 Christian Dietrich CLA 2018-08-14 03:41:32 EDT
any news on this?
Comment 10 Karsten Thoms CLA 2019-01-29 03:30:02 EST
https://github.com/eclipse/xtext-eclipse/pull/955#issuecomment-458441392

Still not working. This prevents whole Eclipse projects primary hosting on GitHub from having fly-by contributions.

Please consider working on that. Also users should be aware that they have to sign CLA before submitting a PR. This should be shown up-front if possible, and best directly sign online at GitHub, or integrate directly to their Eclipse profile.

It must be an almost no-brainer to:
1) See a code problem on GitHub
2) Edit the file online through GitHub web interface
3) Open the PR
4) When no CLA available, prevent submitting the PR and guide to sign one
5) When CLA available, allow submitting the PR
6) Directly sign-off the commit
Comment 11 Denis Roy CLA 2019-01-29 08:57:49 EST
We are working on a new IP validator endpoint via bug 540694.
Comment 12 Christopher Guindon CLA 2019-02-21 13:28:15 EST
Karsten Thoms, 

how do you propose we block someone from submitting a pull request on github?
Comment 13 Karsten Thoms CLA 2019-02-21 15:53:43 EST
Not sure what is possible there. If possible, disable the "Create Pull Request" button when his account is not connected to an Eclipse account or does not have a valid CLA. When there is no Eclipse account, show a link to create one. If no CLA was signed, show a link to it.

Why should someone been blocked to create a PR if he has a valid Eclipse account & CLA? If the preconditions are not fulfilled, there should be guidance.

Just imagine you are someone who sees a source file on Github with a small issue (e.g. a typo), clicks edit and want to submit the change via PR. If the user is not familiar that there is also an Eclipse account & CLA necessary, do this inline when the PR should be created.
Comment 14 Christopher Guindon CLA 2019-02-22 12:05:26 EST
(In reply to Karsten Thoms from comment #13)
> Not sure what is possible there. If possible, disable the "Create Pull
> Request" button when his account is not connected to an Eclipse account or
> does not have a valid CLA. When there is no Eclipse account, show a link to
> create one. If no CLA was signed, show a link to it.

As far as I know, it's not possible to disable "Create Pull Request". If you find a way, please forward the documentation and I will take a look at what we can or should do.

> 
> Why should someone been blocked to create a PR if he has a valid Eclipse
> account & CLA? If the preconditions are not fulfilled, there should be
> guidance.
> 
> Just imagine you are someone who sees a source file on Github with a small
> issue (e.g. a typo), clicks edit and want to submit the change via PR. If
> the user is not familiar that there is also an Eclipse account & CLA
> necessary, do this inline when the PR should be created.

With our new IP validation app, we are looking at improving the feedback that is given for each validation.

For example:
https://accounts.eclipse.org/legal/eca/validation/25

Failure:
SL Corbett (sharonlcorbett@****.ca) did not include the "Signed-off-by footer" which is required for all commits made by a contributor.

I don't think we should block users from submitting a pull request but I do believe that we should include some useful information on the details page on how to solve a failure.
Comment 15 Christopher Guindon CLA 2019-03-18 09:40:24 EDT
The new Github Validation only validates authors not the committers,

I believe this issue is resolved with the new Github App.

(In reply to Christian Dietrich from comment #5)
> There is the same issue with the merge commits done via github web.
> if i want to merge some commits incl merge commits from one branch to
> another the ip validation complains again

We are working on that via Bug 496885 - Github IP validation incorrectly fails on merge commit