palantir / gerrit-ci

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

Multi jobs #16

Closed camushoilingma closed 9 years ago

camushoilingma commented 9 years ago

New changes to add multi jobs

1) Job templates are dynamically created in the following steps:
- JobPanel makes a new template
- User clicks save, each template gets translated to a JenkinsJob object
- All JenkinsJobs are wrapped in a Jobs object and sent to JobsServlet
- Names are <jobType>_<projectName>_RAND

2) Templates can now be added without restarting gerrit
- We now do not depend on the VelocityEngine url resource loader that has issues loading new templates from the jar
- Now JobsServlet will upload xml files and stream them to Velocity which then does the rendering of templates

3)Old and new job configs are now saved but this may be put in a new commit later. All the config files are created in JobsServlet class.
newren commented 9 years ago

A few high-level points:

newren commented 9 years ago

So, I did a 'git log --check master..origin/multi-jobs' and a 'git diff --check master...origin/multi-jobs' and found this interesting src/main/resources/static/projects/fonts/glyphicons-halflings-regular.svg file. Some googling leads me to http://glyphicons.com/license/, which points out that the license not compatible with any open source licenses and can't be uses in open source projects.

We do need to be careful that anything we grab from the internet (code, images, etc.) is either public domain or is both open source and under a license compatible with gerrit-ci (which usually means the exact same license or talk with lawyers to verify it's compatible).

It could be my googling led me to something that just happened to have a similar name, though. Could you verify where these files came from and what their license is?

newren commented 9 years ago

The same three comments on PR #12 continue to apply to your cronJob enable commit.

newren commented 9 years ago

These commits have some conflicts with what is on master. Could you rebase on top of origin/master?

Also, a total aside because I want to complain about github itself: What is up with the utterly moronic github listing of the commits sorted by author date rather than providing a topological sort? "cronJob enable" is the parent of "Added javascriptobjects to hold list of jobs." which is the parent of "Build Jobs from HTML templates", which is the parent of "Support new templates without restarting gerrit". I understand things get fuzzy when there's merges and multiple paths to commits, but this was a totally linear history. This is just retarded, in my opinion.

I also hate how github makes it harder to access commit-by-commit diffs than the overall diff, guiding people to review the entire diff and ignore any slop that might be inbetween. The combination of features really does suggest github is pushing bad practice on people. Is that where stash got it's horrible design mistakes from?

camushoilingma commented 9 years ago

Turns out the glyphicons stuff isn't required so I'll just remove them. As for the three outstanding comments in PR #12 I will change the examples in the next commit and I believe I already updated the error message. Also will fix the formatting code in another commit :)

newren commented 9 years ago

Try running this: for commit in $(git rev-list master..origin/multi-jobs); do git checkout --quiet $commit; ./gradlew --quiet build && echo success $commit || echo failed $commit; done | tee build-results.output

Doing that, I see 5 of the 13 commits failed to build. You can fix that up with an interactive rebase, editing the commits that don't build (the build-results.output file will just list which commits succeed/fail, while your terminal output will be overloaded with the various compilation errors).

newren commented 9 years ago

"fsfsd"? (Also, looks like I was testing before your latest rebase and push, so it's only 4 out of 13 that fail now.)

newren commented 9 years ago

I think it'd be better if 3248dbc (Pass in empty junitPath if junit is not enabled so that builds do not fail. 2015-08-23) were squashed into ecbcd61 (Support new templates without restarting gerrit 2015-08-19).

newren commented 9 years ago

Run git grep -L 'Copyright' -- $(git diff --diff-filter=A --name-only origin/master); it'll give you a list of potential files that need copyright notices . Looks like there are a half-dozen or so java files that need it.

newren commented 9 years ago

Obsoleted by the several rounds of changes to the "WIP" branch, which was eventually merged.