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

Add AddPluginRepository recipe #4247

Merged
merged 7 commits into from
Jun 13, 2024
Merged

Conversation

jdmcmahan
Copy link
Contributor

What's changed?

This PR adds a recipe for AddPluginRepository which can be used to manage a <pluginRepositories> configuration in a Maven POM.

What's your motivation?

Although the AddRepository recipe is sufficient for most use cases, we encountered the need to reference a <pluginRepository> for plugin-specific dependencies. The following example illustrates this:

<pluginRepositories>
    <pluginRepository>
        <id>my-org</id>
        <url>https://maven.myorg.com/repository/</url>
        <name>My Organization's Repository</name>
    </pluginRepository>
</pluginRepositories>

<build>
    <plugins>
        <plugin>
            <groupId>org.apache.maven.plugins</groupId>
            <artifactId>maven-checkstyle-plugin</artifactId>
            <version>3.4.0</version>
            <executions>
                <execution>
                    <id>validate</id>
                    <phase>validate</phase>
                    <goals>
                        <goal>check</goal>
                    </goals>
                </execution>
            </executions>
            <configuration>
                <configLocation>checkstyle.xml</configLocation>
            </configuration>
            <dependencies>
                <dependency>
                    <groupId>org.myorg</groupId>
                    <artifactId>myorg-custom-checkstyle-config</artifactId>
                    <version>1.0.0</version>
                </dependency>
            </dependencies>
        </plugin>
    </plugins>
</build>

Anything in particular you'd like reviewers to focus on?

This change is naïve in the sense that it effectively just copies the logic from the AddRepository class. I'm open to any opportunities to reuse or improve the code.

Anyone you would like to review specifically?

Have you considered any alternatives or workarounds?

AddRepository does not provide an option to create the repository as a plugin repository. Maven itself does not fall back on non-plugin repositories for plugin dependencies.

Any additional context

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've read and applied the recipe conventions and best practices
  • I've used the IntelliJ IDEA auto-formatter on affected files

@timtebeek timtebeek self-requested a review June 12, 2024 16:51
@timtebeek
Copy link
Contributor

Great start @jdmcmahan ! Symmetry with AddPlugin is clear indeed; from a maintainability point of view I'm wondering if we should add an option to AddRepository instead to make that add either a regular or a plugin repository. I'll think that over and update your PR before a merge. Thanks again!

@timtebeek timtebeek added enhancement New feature or request recipe Requested Recipe test provided Already replicated with a unit test, using JUnit pioneer's ExpectedToFail labels Jun 13, 2024
Copy link
Contributor

@timtebeek timtebeek left a comment

Choose a reason for hiding this comment

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

Ended up indeed reusing AddRepository instead @jdmcmahan , just so that we have less copies to maintain over time. I've taken the example test that you had provided and added that into AddRepositoryTest. Thanks again for getting this started; with this option it should be usable already

@jdmcmahan
Copy link
Contributor Author

Ended up indeed reusing AddRepository instead @jdmcmahan , just so that we have less copies to maintain over time. I've taken the example test that you had provided and added that into AddRepositoryTest. Thanks again for getting this started; with this option it should be usable already

Awesome!! Thanks for picking this up!

github-actions[bot]

This comment was marked as outdated.

github-actions[bot]

This comment was marked as off-topic.

github-actions[bot]

This comment was marked as off-topic.

@timtebeek timtebeek merged commit 7182d7b into openrewrite:main Jun 13, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request recipe Requested Recipe test provided Already replicated with a unit test, using JUnit pioneer's ExpectedToFail
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants