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

introdue a new mojo to list all Jenkins plugins in a reactor #463

Closed
wants to merge 1 commit into from

Conversation

jtnord
Copy link
Member

@jtnord jtnord commented Mar 29, 2023

introduces a mojo that will print the artifactID version and location of all projects in the reactor that are Jenkins plugins.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@jtnord jtnord force-pushed the list-plugins-mojo branch 2 times, most recently from 9748664 to 2165985 Compare March 29, 2023 20:55
introduces a mojo that will print the artifactID version and location of
all projects in the reactor that are Jenkins plugins.
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.

OK, I get why you are doing this, but a custom Mojo here seems completely overkill to me. Moreover its output isn't human readable, and I don't think any human would ever want to invoke this Mojo, so it would end up just being an appendage of PCT that lives in another repository, which I don't think is maintainable. Moreover this could be implemented entirely in PCT by listing all modules:

mvn -Dexpression=project.modules -DforceStdout=true -q help:evaluate

And then iterating over each one and getting its packaging, e.g. (for workflow-cps):

mvn -pl dgm-builder -Dexpression=project.packaging -DforceStdout=true -q help:evaluate
mvn -pl lib -Dexpression=project.packaging -DforceStdout=true -q help:evaluate
mvn -pl plugin -Dexpression=project.packaging -DforceStdout=true -q help:evaluate

In this case only plugin returns hpi so that would yield the same result.

I am inclined to close this. If you want me to focus on writing the code I just described above, I am happy to do that. But I wanted to get your consent before closing this PR. What do you think?

@jtnord
Copy link
Member Author

jtnord commented Apr 12, 2023

OK, I get why you are doing this, but a custom Mojo here seems completely overkill to me.
so it would end up just being an appendage of PCT that lives in another repository, which I don't think is maintainable.

I would argue the small amount of code in a maven plugin would be easier to understand than those in the pct.

up just being an appendage of PCT that lives in another repository

agreed - but no different to resolve-test-dependencies which is the bulk of the work in performing the most vital PCT bit (actually updating the plugins/dependencies)

Moreover this could be implemented entirely in PCT by listing all modules:

mvn -Dexpression=project.modules -DforceStdout=true -q help:evaluate

which only works for first level modules and not children of them. Whilst we don't have any currently in the bom or proprietary code there are some Jenkins plugins that have done this in the past and we should not be enforcing a structure onto plugin developers just so that they can use this tooling.
Additionally as this is for local checkout there is nothing preventing creating a top level aggregator and having projects (which may themselves be multi module in the sub directory)

And then iterating over each one and getting its packaging, e.g. (for workflow-cps):

mvn -pl dgm-builder -Dexpression=project.packaging -DforceStdout=true -q help:evaluate
mvn -pl lib -Dexpression=project.packaging -DforceStdout=true -q help:evaluate
mvn -pl plugin -Dexpression=project.packaging -DforceStdout=true -q help:evaluate

In this case only plugin returns hpi so that would yield the same result.

I am inclined to close this. If you want me to focus on writing the code I just described above, I am happy to do that. But I wanted to get your consent before closing this PR. What do you think?

I believe that a single mojo call is easier to understand and should rarely if every need to change. I also concede that putting it in the maven-hpi-plugin is an abuse of this plugin that was more convenient than creating another plugin.

If you desire to inline this into the p-c-t and think it will be cleaner then go for it.

@jtnord
Copy link
Member Author

jtnord commented Apr 13, 2023

was inlined with commit jenkinsci/plugin-compat-tester@e310871

@jtnord jtnord closed this Apr 13, 2023
@jtnord jtnord deleted the list-plugins-mojo branch April 13, 2023 15:21
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.

2 participants