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

[7.11.x] added new groovy script #345

Merged
merged 1 commit into from Oct 14, 2018
Merged

[7.11.x] added new groovy script #345

merged 1 commit into from Oct 14, 2018

Conversation

mbiarnes
Copy link
Contributor

(cherry picked from commit 3003755)

for the compilation of all downstream reps skipping tests and skipping gwt compilation

(cherry picked from commit 3003755)
Copy link
Contributor

@pszubiak pszubiak left a comment

Choose a reason for hiding this comment

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

+1 from a functional point of view.

However, we introduce code duplication. This is hard to maintain in long term. This groovy file is very similar to downstream_pr_jobs.groovy. Wonder if instead of copying code we should refactor it.

@baldimir
Copy link
Member

Looks ok, but agree with @pszubiak that if there is a similar code, it should be refactored, because duplicate code is a pain to maintain.

@mbiarnes
Copy link
Contributor Author

@psiroky @baldimir you're right - but I don't know how to refactor it. What we need now is a fast solution because a lot of people is complaining. For sure I will need some days to find a way how to "refactor" the code.

Copy link
Contributor

@winklerm winklerm left a comment

Choose a reason for hiding this comment

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

Looks ok, just one question.

"optaplanner-wb" : [],
"jbpm-designer" : [],
"jbpm-wb" : []
//"kie-wb-distributions" : [] // kie-wb-distributions is the last repo in the chain
Copy link
Contributor

Choose a reason for hiding this comment

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

Just out of curiosity, why is kie-wb-distributions commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@winklerm good question - this was taken 1:1 from Petr - so we did not change it. There should be no other rep dependent of kie-wb-distributions? (like droolsjbpm-tools)

Copy link

Choose a reason for hiding this comment

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

@winklerm To add my understanding: I believe it's ok to have kie-wb-distributions and droolsjbpm-tools commented out.
The purpose of this groovy script is to generate "compile downstream" jobs is to make sure that changes in repo X don't break any of the repos that depend on X. In this particualr case there are no repos that depend on kie-wb-distributions and on droolsjbpm-tools, so we don't need jobs for those.

"**/target/kie-wb*eap*.war",
"**/target/kie-drools-wb*wildfly*.war",
"**/target/kie-drools-wb*eap*.war",
"**/target/kie-server-*ee6.war",
Copy link

Choose a reason for hiding this comment

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

@mbiarnes you told me that kie-server ee6 is no longer being generated, so it should probably be removed from this "what to archive" section.

label : "kie-rhel7 && kie-mem8g",
ghAuthTokenId : Constants.GITHUB_AUTH_TOKEN,
upstreamMvnArgs : "-B -e -T1C -DskipTests -Dgwt.compiler.skip=true -Dgwt.skipCompilation=true -Denforcer.skip=true -Dcheckstyle.skip=true -Dfindbugs.skip=true -Drevapi.skip=true clean install",
downstreamMvnGoals : "-B -e -nsu -fae clean install -Dfull=true -DskipTests -Dgwt.compiler.skip=true -Dgwt.skipCompilation=true",
Copy link

Choose a reason for hiding this comment

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

Since we won't be running any tests in these "just compile" jobs, I would suggest adding -T1C into the downstreamMvnGoals - here maven parallelism shouldn't hurt, on the contrary it will make the jobs run faster.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1; adding -T1C seems to be a very good idea.

Copy link
Contributor

@pszubiak pszubiak left a comment

Choose a reason for hiding this comment

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

@mbiarnes @baldimir I'm fine with merging it "as is" and refactor later.

My other concern is that these two PRs will increase usage of GitHub PR builder plug-in by ~50%. My impression is that the more we use ghpr plug-in the more often Jenkins hangs due to bug identified in it (JENKINS-19123).

Wonder if it makes sense to run these jobs on kie-jenkins until ghpr plug-in issue is solved? Doing that we provide even more chaos but we are limiting the negative impact of this new jobs on normal downstream and PRs jobs. What do you think?

label : "kie-rhel7 && kie-mem8g",
ghAuthTokenId : Constants.GITHUB_AUTH_TOKEN,
upstreamMvnArgs : "-B -e -T1C -DskipTests -Dgwt.compiler.skip=true -Dgwt.skipCompilation=true -Denforcer.skip=true -Dcheckstyle.skip=true -Dfindbugs.skip=true -Drevapi.skip=true clean install",
downstreamMvnGoals : "-B -e -nsu -fae clean install -Dfull=true -DskipTests -Dgwt.compiler.skip=true -Dgwt.skipCompilation=true",
Copy link
Contributor

Choose a reason for hiding this comment

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

+1; adding -T1C seems to be a very good idea.

@jhrcek jhrcek changed the title added new groovy script [7.11.x] added new groovy script Oct 12, 2018
Copy link

@jhrcek jhrcek left a comment

Choose a reason for hiding this comment

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

Looks ok to me. Mu understanding is that these new jobs will be run on the old jenkins instance untill the issues with jenkinks github plugins is resolved. Do I have it right?

@pszubiak
Copy link
Contributor

@jhrcek Yes, that's the idea. Once the issue with ghpr plug-in is solved it will land on rhba-jenkins. The new functionality first will be introduced for the master branch and if everything is fine then support for 7.11.x will be added.

Copy link
Member

@mareknovotny mareknovotny left a comment

Choose a reason for hiding this comment

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

+1 for this experiment

@mareknovotny mareknovotny merged commit e8c728f into kiegroup:7.11.x Oct 14, 2018
@mbiarnes mbiarnes deleted the compilation-downstream-7.11.x branch October 15, 2018 06:32
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.

None yet

6 participants