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

Support PCT for multi-module Maven projects (II) #689

Merged
merged 4 commits into from
Mar 27, 2023

Conversation

jglick
Copy link
Member

@jglick jglick commented Mar 27, 2023

Subsumes #685 but should allow jenkins-infra/repository-permissions-updater#3210 to be reverted, cleaning up technical debt from #612. Local testing seems fine using both a clean working copy and a clean local repo:

mvn clean test -Dtest=CpsDefaultGroovyMethodsTest

@jglick jglick requested a review from a team as a code owner March 27, 2023 16:20
@jglick jglick requested review from dwnusbaum and basil March 27, 2023 16:20
@jglick jglick changed the title Support PCT for multi-module Maven projects Support PCT for multi-module Maven projects (II) Mar 27, 2023
basil added a commit to basil/bom that referenced this pull request Mar 27, 2023
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

Failed PCT against the multimodule project branch. Note that #685 passes.

What goal are you trying to achieve by continuing to dig into this?

@jglick
Copy link
Member Author

jglick commented Mar 27, 2023

mvn test worked but mvn surefire:test (even after previous compilation) did not; I think I have fixed that.

Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

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

The latest revision passes local PCT testing for me on the multimodule project branch. Still not sure it was worth the effort.

@dwnusbaum dwnusbaum merged commit e2e99a4 into jenkinsci:master Mar 27, 2023
@jglick jglick deleted the multimodule-simplified branch March 27, 2023 20:53
@jglick
Copy link
Member Author

jglick commented Mar 27, 2023

@basil
Copy link
Member

basil commented Mar 28, 2023

Breaks the master branch of PCT, unlike my original proposal from #685 (which works on both the master and multimodule branches of PCT). Can be seen by running PLUGINS=workflow-cps TEST=InjectedTest bash local-test.sh in jenkinsci/bom#1903.

I suggest that you either figure out how to get your code to work on both PCT branches or that you accept my original submission (which does work on both branches), even if you consider it "undesirable". You can always switch back to your code once the PCT multimodule project has been integrated.

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.

3 participants