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

Use ExternalRules enforcer rule #27294

Merged
merged 1 commit into from
Aug 30, 2022
Merged

Use ExternalRules enforcer rule #27294

merged 1 commit into from
Aug 30, 2022

Conversation

gastaldi
Copy link
Contributor

@gastaldi gastaldi commented Aug 15, 2022 •
edited
Loading

This moves the rules to an external rule file to allow reusing enforcer rules in another modules.
For more information: https://github.com/gastaldi/enforcer-rules#externalrules

PS: The ExternalRules rule is tracked in MENFORCER-422 and will be migrated to the maven-enforcer-plugin upstream
repository in this PR:

@gastaldi gastaldi requested review from gsmet and ppalaga August 15, 2022 19:59
@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Aug 15, 2022
@gastaldi gastaldi force-pushed the enforcer_rules branch 2 times, most recently from 6320bd2 to 839e53b Compare August 16, 2022 03:28
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
gsmet
gsmet reviewed Aug 23, 2022
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Nice. I added two comments. Can we discuss them?

independent-projects/enforcer-rules/pom.xml
<!--
Supported Maven versions, interpreted as a version range (Also defined in build-parent)
-->
<supported-maven-versions>[3.6.2,)</supported-maven-versions>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure it makes sense to define this one here. Rationale: the supported Maven version really depends on the project you are currently building and Quarkus has nothing to do with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, isn't that important for our Maven plugins?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be interested to get <supported-maven-versions> in Camel Quarkus. I think there will inevitably be entries in this rules file that we won't be wanting to enforce. A XSLT pre-filtering will solve all such issues.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if we split the rules into several files (one for each rule type)?
e.g.

  • quarkus-require-java-version.xml;
  • quarkus-require-maven-version.xml;
  • quarkus-banned-dependencies.xml

Copy link
Contributor

Choose a reason for hiding this comment

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

What if we split the rules into several files

Hm... how do I get notified that there is a new rule file?

Copy link
Contributor Author

@gastaldi gastaldi Aug 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We could still have a quarkus-default.xml with <enforcerRules> to each of these individual files which if you use it you don't need to bother about new rule files.

independent-projects/enforcer-rules/src/main/resources/enforcer-rules/default-quarkus.xml Outdated
<rules>
<dependencyConvergence/>
<requireJavaVersion>
<version>[${maven.compiler.release},)</version>
Copy link
Member

Choose a reason for hiding this comment

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

Same for this one. I think I would refrain from adding it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not 100% sure about this to be honest. If not here, I'd expect that at least this rule would need to be included in each project, which may lead to values that are not expected in Quarkus (eg. Extensions being compiled/released using Java 17+ instead of 11)

Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above: I'd be interested to get <supported-maven-versions> in Camel Quarkus.

@gastaldi gastaldi force-pushed the enforcer_rules branch 2 times, most recently from f2903f9 to 27a6be3 Compare August 23, 2022 15:47
@quarkus-bot

This comment has been minimized.

Sign in to view
@gastaldi gastaldi force-pushed the enforcer_rules branch 3 times, most recently from 3a41508 to 84e77da Compare August 23, 2022 16:29
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
gsmet
gsmet reviewed Aug 26, 2022
View reviewed changes
independent-projects/enforcer-rules/pom.xml
<!--
Supported Maven versions, interpreted as a version range (Also defined in build-parent)
-->
<supported-maven-versions>[3.6.2,)</supported-maven-versions>
Copy link
Member

Choose a reason for hiding this comment

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

If we move this to the global parent pom, wouldn't it be available everywhere?

Copy link
Contributor Author

@gastaldi gastaldi Aug 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

What global parent pom are you talking about? quarkus-enforcer-rules has jboss-parent as the parent because it's an independent project

@gastaldi
Use ExternalRules enforcer rule
ce2b969 
- Fixes quarkusio#24880

This moves the rules to an external rule file to allow reusing enforcer rules in another modules.
For more information: https://github.com/gastaldi/enforcer-rules#externalrules

PS: The ExternalRules rule is tracked in [MENFORCER-422](https://issues.apache.org/jira/browse/MENFORCER-422)  will be migrated to the maven-enforcer-plugin upstream
repository in this PR:

- apache/maven-enforcer#180
gsmet
gsmet approved these changes Aug 30, 2022
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

I'll deal with adapting the Jakarta script.

@gsmet gsmet merged commit 1acb786 into quarkusio:main Aug 30, 2022
@quarkus-bot quarkus-bot bot added this to the 2.13 - main milestone Aug 30, 2022
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Aug 30, 2022
@ppalaga ppalaga mentioned this pull request Sep 5, 2022
Closed
@gastaldi gastaldi deleted the enforcer_rules branch September 5, 2022 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ppalaga ppalaga ppalaga approved these changes

@gsmet gsmet gsmet approved these changes

Assignees
No one assigned
Labels
area/build area/dependencies Pull requests that update a dependency file kind/enhancement New feature or request
Projects
None yet
Milestone
2.13.0.CR1
Development

Successfully merging this pull request may close these issues.

Share the set of banned dependencies and their replacements with Quarkus ecosystem
3 participants
@gastaldi @gsmet @ppalaga