Incorrect CODEOWNERS file created for Priority Sorter plugin and TestNG plugin

MarkEWaite commented 1 year ago

What version of OpenRewrite are you using?

I am using

Maven plugin v5.4.2

I am not sure of the other versions, but the maven output indicates Maven plugin 5.4.2

How are you running OpenRewrite?

Running the Maven plugin on a single module project https://github.com/jenkinsci/priority-sorter-plugin I see an unexpected incorrect syntax in the generated CODEOWNERS file. The name of the developers team that was initially created by the OpenRewrite recipe was missing a "-" character.

The initial file was:

* @jenkinsci/prioritysorter-plugin-developers

The updated file was:

* @jenkinsci/priority-sorter-plugin-developers

I am using the Maven plugin with the command line as described in the pull request:

$ mvn -U org.openrewrite.maven:rewrite-maven-plugin:run \
      -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-jenkins:RELEASE \
      -Drewrite.activeRecipes=org.openrewrite.jenkins.github.AddTeamToCodeowners

Similar error with the TestNG plugin pull request, though in the case of the TestNG plugin, it may be due to the uncommon duplication of the word "plugin" in the repository name

What is the smallest, simplest way to reproduce the problem?

Run the provided command line after removing the corrected CODEOWNERS file from the repository.

What did you expect to see?

I expected the CODEOWNERS file to contain the correct team name * @jenkinsci/priority-sorter-plugin-developers as it did on the other pull requests that I created. Refer to the other pull requests:

The updated file was:

* @jenkinsci/priority-sorter-plugin-developers

What did you see instead?

The initial file was:

* @jenkinsci/prioritysorter-plugin-developers

What is the full stack trace of any errors you encountered?

No stack trace or other indication of error

Are you interested in contributing a fix to OpenRewrite?

I lack the skills to contribute a fix.

sghill commented 1 year ago

Thanks for the report @MarkEWaite! Apologies for having to correct a few PRs.

I think we'll need to implement an override layer to make this work for all the OSS plugins.

If you're interested in what's going on -

I took the CODEOWNERS archetype to mean there was a strong convention around the relationship between artifactId and team name. The first implementation took this literally, pulling the artifactId from pom.xml and adding -plugin-developers on the end of it.

This does seem to work for most plugins, but #42 introduced some logic to handle a few edge cases I saw as I began to roll this out:

Sometimes the artifactId had uppercase letters, but the team name did not
Sometimes the artifactId was hyphenated, but the team name was not
Sometimes the artifactId ended in -plugin, but I had not found a team named -plugin-plugin-developers

And now that you've found and generously reported a couple more I think it probably makes sense to do a one-time pass through all the teams and test that they're mapping to valid team names. Going forward I think we can expect the convention of artifactId:artifactId-plugin-developers will cover new plugins.

MarkEWaite commented 1 year ago

Thanks for the description. I agree that there is a strong convention around the relationship between artifactId and team name. I'm not sure that it is worth handling any more edge cases than you already handle.

The two failures that I see are older plugins where I'm unwilling to do the work necessary to make them comply with the relationship between artifactId and team name. It is easy enough to fix the CODEOWNERS file interactively for those few that do not follow the convention. No problem for me if you choose to close this issue.

openrewrite / rewrite-jenkins