git-plugin icon indicating copy to clipboard operation
git-plugin copied to clipboard

JENKINS-70331: Always honor useExistingAccountWithSameEmail

Open reddwarf69 opened this issue 2 years ago • 2 comments

useExistingAccountWithSameEmail makes sense on its own, independently of how a new account may be created if required.

JENKINS-70331 - Always honor useExistingAccountWithSameEmail

In general, I'm not convinced about the whole separation in https://github.com/jenkinsci/git-plugin/blob/756dc6d7855cd8b80041bd79926e7dde84aa3d90/src/main/java/hudson/plugins/git/GitChangeSet.java#L443 based on createAccountBasedOnEmail. I think there are two independent things:

  • Search
  • Creation if not found And "createAccountBasedOnEmail" whould only affect "Creation if not found".

By having the two big branches the "Search" gets duplicated. I'm not trying to fix this here, but I am adding to one of the branches logic that is already in the other one and should IMHO have been in both.

Checklist

  • [x] I have read the CONTRIBUTING doc
  • [x] I have referenced the Jira issue related to my changes in one or more commit messages
  • [x] I have added tests that verify my changes
  • [x] Unit tests pass locally with my changes
  • [ ] I have added documentation as necessary
  • [x] No Javadoc warnings were introduced with my changes
  • [x] No spotbugs warnings were introduced with my changes
  • [x] I have interactively tested my changes

I don't think there is any doc change strictly required for this PR. But the docs regarding all this could, in general, be improved.

Types of changes

  • [ ? ] Bug fix (non-breaking change which fixes an issue)
  • [ ? ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

I guess this could potentially break for somebody? Not exactly sure how.

reddwarf69 avatar Dec 23 '22 15:12 reddwarf69

My understanding is that the only test failing is https://github.com/jenkinsci/git-plugin/pull/1382/checks?check_run_id=10352017446, and that it is unrelated to this change. Am I right?

reddwarf69 avatar Dec 30 '22 09:12 reddwarf69

My understanding is that the only test failing is https://github.com/jenkinsci/git-plugin/pull/1382/checks?check_run_id=10352017446, and that it is unrelated to this change. Am I right?

Yes, that is correct. When tests run in parallel (as they do on ci.jenkins.io) there may be times when two tests are making a change to the git global configuration. Command line git locks the global configuration file (as it should) when it is being modified. I'll run the job again after my current tasks preparing the git plugin 5.0.0 release are complete.

Git client plugin 4.0.0 has released. Git plugin 5.0.0 release process is running now.

MarkEWaite avatar Dec 30 '22 13:12 MarkEWaite