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

Add a repo-branch argument to with-git which gets the latest revision… #46

Merged
merged 3 commits into from Sep 10, 2015

Conversation

kennyjwilli
Copy link
Contributor

… of the given branch and passes it on. This is useful for those using Gitflow because it is more common to checkout the develop branch instead of the master branch.

@flosell
Copy link
Owner

flosell commented Sep 9, 2015

Thanks for your pull request!

I initially designed this with pipelines in mind that have a wait-for-git step before the with-git. In those cases, wait-for-git would take care of finding the next revision on a branch and with-git is just using the :revision argument from args.

I'm guessing you just want to build the latest commit on a branch. That sounds reasonable to me.

I just have one doubt: Your pull request would introduce a behavior where with-git is normally using the given revision, except if it's being called with three arguments. I'm a bit worried that this isn't exactly intuitive to users.

How about we give put this into a separate function to make it explicit?

I think we can simplify the implementation as well: we don't actually need to find the current-revision since the way checkout-and-execute works, it should also allow a branch as a the revision argument.

Also, if you could add a test for your feature that would be awesome! Prevents me from accidently breaking it :)
This should get you started: https://github.com/flosell/lambdacd/blob/master/test/clj/lambdacd/steps/git_test.clj#L162
If you need help, feel free to reach out.

Last but not least, you don't need to bump the version in project.clj, it's already pointing to the next release.

@kennyjwilli
Copy link
Contributor Author

Your assumption is correct: I want to build the latest commit from a branch.

Moved and updated the feature to with-git-branch. git checkout does not need a -b argument when checking out a branch, so no changes were made to checkout-and-execute.

What exactly are you looking for in the test? The only thing that could break is the checkout-and-execute function which already has a test, correct?

@flosell
Copy link
Owner

flosell commented Sep 10, 2015

Looks good!

I would maybe add an additional test that tests your new feature, that it checks out a repository on that branch and calls steps with a :cwd that points to it. Is probably going to look very similar to with-git-test. (I probably should have linked to this one in the first place). (And ignore the TODO on top of that test, thinking about it now I don't think that TODO was a good idea and I'll just get rid of it)

@kennyjwilli
Copy link
Contributor Author

Yeah that TODO was throwing me off for writing a test case 😕. Added test cases for checking out a specific branch, checking out a repo with an unknown uri, and checking out a repo with a known uri but unknown branch.

flosell added a commit that referenced this pull request Sep 10, 2015
Add a repo-branch argument to with-git which gets the latest revision…
@flosell flosell merged commit b6dc8f4 into flosell:master Sep 10, 2015
@flosell
Copy link
Owner

flosell commented Sep 10, 2015

All good, merged! Thanks for the contribution. Also, I got rid of that confusing TODO.

@flosell
Copy link
Owner

flosell commented Sep 12, 2015

Released in version 0.5.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants