greenkeeperio / greenkeeper-lockfile

:lock: Your lockfile, up to date, all the time
https://greenkeeper.io
183 stars 73 forks source link

Jenkins: branchName splitting prevents detection of greenkeeper #145

Open kaylee42 opened 6 years ago

kaylee42 commented 6 years ago

In the jenkins ci-services file, branchName is set by performing some manipulation magic on the GIT_BRANCH env variable. However, the result of this manipulation is the prevention of recognizing a branch as a greenkeeper branch.

For instance: my GIT_BRANCH variable is greenkeeper/initial. Because of the drop in the above referenced line, branchName gets set to initial rather than the correct greenkeeper/initial, so it's falls into this conditional and I get an 'initial' is not a Greenkeeper branch error.

This was fixed in the (apparently now defunct ?? ) #73 by removing the branchName manipulation code. I'm happy to submit a new PR which removes this manipulation, but wasn't sure if it would be better for me to pick up #73 and complete the code review comments (though I'm not sure about the purpose of some of the other changes). Please advise and I'll get a PR in :)

espy commented 6 years ago

Hi Kaylee, thanks for the PR offer! If you do want to tackle this, here’s why the mangling happens: some customers use a git plugin for Jenkins that also prepends something to the branch name, the removal of which makes sense for them, but breaks everything for users like you, who use the other git plugin (apparently there’s two), which doesn’t prepend anything.

Currently, the branch prefix used by by Greenkeeper is always greenkeeper/ or greenkeeper-, so it’s safe to drop the first part of branchName if it doesn’t match greenkeeper, or drop everything before greenkeeper, both should work.

So it would make sense to pick up #73 again, but only do the drop with the condition described above. If you don’t have time, we'll fit it in next week.

Have a good weekend 👋

kaylee42 commented 6 years ago

@espy thanks! Just put in a PR for this #148 (I pulled down changes from #73 but couldn't push to that branch)