Don't overwrite user's email, if it already exists#4
Conversation
|
Jenkins » github-oauth-plugin #16 SUCCESS |
|
Jenkins » github-oauth-plugin #17 SUCCESS |
|
Jenkins » github-oauth-plugin #18 SUCCESS |
|
Can we please get this merged? Our users are complaining about this! |
|
@bbbco, good news! You don't have to wait for this to be merged and for a release to be made including it. You can fork the plugin, merge in this change, and deploy your custom build. Your users could be made happy TODAY! |
|
Understood, however, we are not a Java based shop. No one has used Maven here, and we don't have resources available to learn how to do so. In an ideal world, this would be as simple as you make it sound, but unfortunately, its not :( |
There was a problem hiding this comment.
Looks like this if statement is the beef. Please avoid changing so much indenting. Using Util.fixEmptyAndTrim here would make the check much more robust.
There was a problem hiding this comment.
@johnou It was a deliberate change to fix an evil some-spaces-some-tabs case.
I also split my change into two commits (https://github.com/jenkinsci/github-oauth-plugin/pull/4/commits) - one for indentation only and one for the concrete fix.
Maybe I should have used two separate pull requests, but I really think the indentation change is negligible and this pull request should be accepted.
There was a problem hiding this comment.
If the Jenkins user's email is already filled in, in most cases it's the email the user wants, and overwriting it in each login is unexpected and undesired.