palantir / gerrit-ci

Plugin for Gerrit enabling self-service continuous integration workflows with Jenkins.
Apache License 2.0
18 stars 9 forks source link

cronJob enable #12

Closed camushoilingma closed 9 years ago

newren commented 9 years ago

I think this commit is mostly fine, and like the idea of handling most the big idea stuff in later commits.

However, it looks like some of my questions/concerns are still outstanding; in particular: (1) the cron example has both a slash and a dash in the hours field, which are both disallowed, (2) the integers or 'H' comment, (3) and I'm still not following the 'Please note' comment.

To answer my own earlier question, I don't think any branch regex should be specified by the user with cron jobs because these are triggered solely by the passage of time and thus are independent of any branch/tag. Users should be required to check out the branch (or branches or tags or whatever) that they are interested in from their scripts. It would be useful to verify what Jenkins does, though (does it just leave the user with whatever branch the previous build on that machine happens to have checked out?)

newren commented 9 years ago

Already applied as part of "wip" (which should have been renamed before merging, but oh well...)