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

Incorrect CODEOWNERS file created for Priority Sorter plugin and TestNG plugin #47

Closed
MarkEWaite opened this issue Aug 27, 2023 · 2 comments · Fixed by #48
Closed

Incorrect CODEOWNERS file created for Priority Sorter plugin and TestNG plugin #47

MarkEWaite opened this issue Aug 27, 2023 · 2 comments · Fixed by #48
Assignees
Labels
bug Something isn't working

Comments

@MarkEWaite
Copy link
Contributor

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.

@MarkEWaite MarkEWaite added the bug Something isn't working label Aug 27, 2023
@sghill
Copy link
Collaborator

sghill commented Aug 27, 2023

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
Copy link
Contributor Author

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.

@timtebeek timtebeek moved this to Ready to Review in OpenRewrite Aug 30, 2023
@github-project-automation github-project-automation bot moved this from Ready to Review to Done in OpenRewrite Aug 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants