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.

Sign up for GitHub

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

Jump to bottom

Flatten repositories from JEP-229 plugins #802

Merged
merged 5 commits into from
Jul 6, 2023
Merged

Flatten repositories from JEP-229 plugins #802

merged 5 commits into from
Jul 6, 2023

Conversation

Vlatombe
Copy link
Member

@Vlatombe Vlatombe commented Jul 6, 2023

This workarounds mojohaus/flatten-maven-plugin#362

and strips <repositories> from deployed pom for plugins using JEP-229.

Testing done

Submitter checklist
Preview Give feedback

@Vlatombe
Flatten repositories from JEP-229 plugins
219e268 
This workarounds mojohaus/flatten-maven-plugin#362

and strips <repositories> from deployed pom for plugins using JEP-229
@Vlatombe Vlatombe requested a review from jglick July 6, 2023 11:56
jglick
jglick approved these changes Jul 6, 2023
View reviewed changes
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Would also be great to have an IT covering the basic build cycle of a JEP-229-based plugin unlike

plugin-pom/src/it/incrementals-and-plugin-bom/.mvn/maven.config

Lines 1 to 2 in e46655d

-Pconsume-incrementals
-Pmight-produce-incrementals
and the validation script could do some stuff like assert that the top-level POM elements are only those in a specified list.

jglick
jglick reviewed Jul 6, 2023
View reviewed changes
pom.xml Show resolved Hide resolved
@Vlatombe
Copy link
Member Author

Vlatombe commented Jul 6, 2023

IT covering the basic build cycle of a JEP-229-based plugin

I'll see what I can do

@Vlatombe Vlatombe requested a review from jglick July 6, 2023 12:54
@Vlatombe Vlatombe added the bug label Jul 6, 2023
jglick
jglick approved these changes Jul 6, 2023
View reviewed changes
Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

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

Nice!

src/it/cd-plugin/src/main/java/test/SampleRootAction.java Outdated Show resolved Hide resolved
src/it/cd-plugin/src/test/java/test/SampleRootActionTest.java Outdated Show resolved Hide resolved
src/it/cd-plugin/src/test/resources/test/expected.txt Outdated Show resolved Hide resolved
src/it/cd-plugin/postbuild.groovy
assert installedPom.text.contains("<scm>")
assert !installedPom.text.contains("<properties>")
assert !installedPom.text.contains("<dependencyManagement>")
assert !installedPom.text.contains("<repositories>")
Copy link
Member

Choose a reason for hiding this comment

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

(the point of the PR, FTR)

src/it/cd-plugin/invoker.properties Outdated Show resolved Hide resolved
@Vlatombe Vlatombe merged commit 4f81ec1 into jenkinsci:master Jul 6, 2023
@Vlatombe Vlatombe deleted the flatten-repositories branch July 6, 2023 15:21
@basil
Copy link
Member

basil commented Jul 11, 2023

Does this need to be side-ported to https://github.com/jenkinsci/pom?

Vlatombe added a commit to Vlatombe/pom that referenced this pull request Jul 12, 2023
@Vlatombe
Flatten <repositories> part of deployed pom when using JEP-229
9dfe6c4 
Counterpart of jenkinsci/plugin-pom#802,
workarounds mojohaus/flatten-maven-plugin#362
Vlatombe added a commit to Vlatombe/pom that referenced this pull request Jul 12, 2023
@Vlatombe
Flatten <repositories> part of deployed pom when using JEP-229
30cb7f4 
Counterpart of jenkinsci/plugin-pom#802,
workarounds mojohaus/flatten-maven-plugin#362
@Vlatombe Vlatombe mentioned this pull request Jul 12, 2023
Merged
@Vlatombe
Copy link
Member Author

Good point, jenkinsci/pom#464

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jglick jglick jglick approved these changes

Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Vlatombe @basil @jglick