Skip to content

Conversation

@MarkEWaite
Copy link
Contributor

@MarkEWaite MarkEWaite commented Feb 5, 2018

Christian Halstrick of the JGit project sat with me for a day at the Jenkins Hackfest after FOSDEM. We reviewed the JGit implementation and made improvements while capturing action items.

  • automated tests pass
  • interactive tests pass
  • code review

@reviewbybees

@MarkEWaite MarkEWaite changed the title Use JGit exactRef instead of getRef to resolve precise ref JGit improvements from code review with Christian Halstrick Feb 5, 2018
@MarkEWaite MarkEWaite changed the title JGit improvements from code review with Christian Halstrick JGit improvements from Christian Halstrick review Feb 5, 2018
@MarkEWaite MarkEWaite force-pushed the jgit-improvements-with-christian-halstrick branch 4 times, most recently from 143f4ca to 1c79596 Compare February 9, 2018 16:01
@MarkEWaite MarkEWaite force-pushed the jgit-improvements-with-christian-halstrick branch 2 times, most recently from 8950fbd to 2f2db17 Compare February 17, 2018 21:17
@MarkEWaite MarkEWaite force-pushed the jgit-improvements-with-christian-halstrick branch 7 times, most recently from 3d56fbb to 7352bf2 Compare March 13, 2018 17:14
@MarkEWaite MarkEWaite force-pushed the jgit-improvements-with-christian-halstrick branch from 7352bf2 to 4c5fd6b Compare March 22, 2018 05:03
@MarkEWaite MarkEWaite force-pushed the jgit-improvements-with-christian-halstrick branch 5 times, most recently from 8ddabe9 to 48c707b Compare April 10, 2018 02:24
@MarkEWaite MarkEWaite force-pushed the jgit-improvements-with-christian-halstrick branch from 48c707b to a747dd7 Compare April 17, 2018 12:45
The JGit implementation has deprecated getRef and replaced it with
two implementations, one for exactRef() and one for findRef().  In
this case, the ref is being formed into an exact reference, so the
exactRef() call is the correct choice.
Remove redundant check for null ref in branch checkout

Also rename doCheckout methods to more clearly say what they do.

Add null ref check to tests
Reduce unnecessary reimplementation of LsRemoteCommand in the
JGitAPIImpl source code.
@MarkEWaite MarkEWaite force-pushed the jgit-improvements-with-christian-halstrick branch from a747dd7 to 2794867 Compare April 20, 2018 18:36
Copy link

@Wadeck Wadeck left a comment

Choose a reason for hiding this comment

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

🐝 Even without lots of knowledge in that area, the changes seem to be reasonnable.

@MarkEWaite MarkEWaite merged commit 0bb3b57 into jenkinsci:master Apr 24, 2018
@MarkEWaite MarkEWaite deleted the jgit-improvements-with-christian-halstrick branch April 24, 2018 16:07
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.

2 participants