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

feat: Determine SCM and parse clone url in GitProvenance for more accurate path/origin/organiation/groups #4367

Closed
wants to merge 23 commits into from

Conversation

pstreef
Copy link
Contributor

@pstreef pstreef commented Aug 1, 2024

What's changed?

Determine the SCM from a list of known urls and any manually registered SCMs. The SCM is then used to parse a typed CloneUrl which in turn give information about a repository. Depending on the SCM there are some expectations about the clone url that must be met.

What's your motivation?

To determine the repository origin, path, organization, project and group path (for GitLab) is tricky, especially with self-hosted SCMs. Doing this in 1 place makes it more maintainable.

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

Any url patterns that are missed

Anyone you would like to review specifically?

@bryceatmoderne

Have you considered any alternatives or workarounds?

Implementing this in higher level code/places.

Any additional context

This is going to be used directly by Moderne CLI, but should be useful for other projects as well.

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

Determine the SCM from a list of known urls and any manually registered SCMs. The SCM is then used to determine ScmUrlComponents which in turn give information about a repository.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@pstreef pstreef changed the title feat: Determine SCM in GitProvenance for more accurate path estimation feat: Determine SCM and parse clone url in GitProvenance for more accurate path/origin/organiation/groups Aug 2, 2024
pstreef and others added 2 commits August 2, 2024 09:50
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…eUrl.java

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@pstreef pstreef marked this pull request as ready for review August 2, 2024 07:50
pstreef and others added 2 commits August 2, 2024 09:50
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@timtebeek
Copy link
Contributor

Resolved these issues with a quick recipe run from the IntelliJ IDEA plugin through

type: specs.openrewrite.org/v1beta/recipe
name: Scratch rewrite.yml
recipeList:
  - org.openrewrite.java.format.EmptyNewlineAtEndOfFile

The full list of checks on every pull request is maintained here:

---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.recipes.OpenRewriteBestPracticesSubset
displayName: OpenRewrite best practices
description: Best practices for OpenRewrite recipe development.
recipeList:
- org.openrewrite.recipes.JavaRecipeBestPracticesSubset
- org.openrewrite.recipes.RecipeTestingBestPracticesSubset
- org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset
#- org.openrewrite.java.OrderImports
- org.openrewrite.java.format.EmptyNewlineAtEndOfFile
- org.openrewrite.staticanalysis.InlineVariable
- org.openrewrite.staticanalysis.MissingOverrideAnnotation
- org.openrewrite.staticanalysis.UseDiamondOperator
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.recipes.JavaRecipeBestPracticesSubset
displayName: Java Recipe best practices
description: Best practices for Java recipe development.
preconditions:
- org.openrewrite.java.search.FindTypes:
fullyQualifiedTypeName: org.openrewrite.Recipe
checkAssignability: true
recipeList:
- org.openrewrite.java.recipes.BlankLinesAroundFieldsWithAnnotations
- org.openrewrite.java.recipes.ExecutionContextParameterName
- org.openrewrite.java.recipes.MissingOptionExample
- org.openrewrite.java.recipes.RecipeEqualsAndHashCodeCallSuper
- org.openrewrite.java.recipes.UseTreeRandomId
- org.openrewrite.staticanalysis.NeedBraces
#- org.openrewrite.staticanalysis.RemoveSystemOutPrintln
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.recipes.RecipeTestingBestPracticesSubset
displayName: Recipe testing best practices
description: Best practices for testing recipes.
preconditions:
- org.openrewrite.java.search.FindTypes:
fullyQualifiedTypeName: org.openrewrite.test.RewriteTest
checkAssignability: true
recipeList:
- org.openrewrite.java.recipes.RewriteTestClassesShouldNotBePublic
#- org.openrewrite.java.recipes.SelectRecipeExamples
- org.openrewrite.java.recipes.SourceSpecTextBlockIndentation
- org.openrewrite.java.testing.cleanup.RemoveTestPrefix
- org.openrewrite.java.testing.cleanup.TestsShouldNotBePublic
- org.openrewrite.staticanalysis.NeedBraces
- org.openrewrite.staticanalysis.RemoveSystemOutPrintln
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset
displayName: Recipe nullability best practices
description: Use OpenRewrite internal nullability annotations; drop JetBrains annotations; use `package-info.java` instead.
recipeList:
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: org.jetbrains.annotations.Nullable
newFullyQualifiedTypeName: org.openrewrite.internal.lang.Nullable
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: jakarta.annotation.Nullable
newFullyQualifiedTypeName: org.openrewrite.internal.lang.Nullable
- org.openrewrite.java.RemoveAnnotation:
annotationPattern: '@org.jetbrains.annotations.NotNull'
- org.openrewrite.java.RemoveAnnotation:
annotationPattern: '@jakarta.annotation.Nonnull'

I'm considering adding that to a root rewrite.yml to make it easier to run these recipes on your local machine. Thoughts?

@pstreef
Copy link
Contributor Author

pstreef commented Aug 2, 2024

Resolved these issues with a quick recipe run from the IntelliJ IDEA plugin through

type: specs.openrewrite.org/v1beta/recipe
name: Scratch rewrite.yml
recipeList:
  - org.openrewrite.java.format.EmptyNewlineAtEndOfFile

The full list of checks on every pull request is maintained here:

---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.recipes.OpenRewriteBestPracticesSubset
displayName: OpenRewrite best practices
description: Best practices for OpenRewrite recipe development.
recipeList:
- org.openrewrite.recipes.JavaRecipeBestPracticesSubset
- org.openrewrite.recipes.RecipeTestingBestPracticesSubset
- org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset
#- org.openrewrite.java.OrderImports
- org.openrewrite.java.format.EmptyNewlineAtEndOfFile
- org.openrewrite.staticanalysis.InlineVariable
- org.openrewrite.staticanalysis.MissingOverrideAnnotation
- org.openrewrite.staticanalysis.UseDiamondOperator
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.recipes.JavaRecipeBestPracticesSubset
displayName: Java Recipe best practices
description: Best practices for Java recipe development.
preconditions:
- org.openrewrite.java.search.FindTypes:
fullyQualifiedTypeName: org.openrewrite.Recipe
checkAssignability: true
recipeList:
- org.openrewrite.java.recipes.BlankLinesAroundFieldsWithAnnotations
- org.openrewrite.java.recipes.ExecutionContextParameterName
- org.openrewrite.java.recipes.MissingOptionExample
- org.openrewrite.java.recipes.RecipeEqualsAndHashCodeCallSuper
- org.openrewrite.java.recipes.UseTreeRandomId
- org.openrewrite.staticanalysis.NeedBraces
#- org.openrewrite.staticanalysis.RemoveSystemOutPrintln
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.recipes.RecipeTestingBestPracticesSubset
displayName: Recipe testing best practices
description: Best practices for testing recipes.
preconditions:
- org.openrewrite.java.search.FindTypes:
fullyQualifiedTypeName: org.openrewrite.test.RewriteTest
checkAssignability: true
recipeList:
- org.openrewrite.java.recipes.RewriteTestClassesShouldNotBePublic
#- org.openrewrite.java.recipes.SelectRecipeExamples
- org.openrewrite.java.recipes.SourceSpecTextBlockIndentation
- org.openrewrite.java.testing.cleanup.RemoveTestPrefix
- org.openrewrite.java.testing.cleanup.TestsShouldNotBePublic
- org.openrewrite.staticanalysis.NeedBraces
- org.openrewrite.staticanalysis.RemoveSystemOutPrintln
---
type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.recipes.RecipeNullabilityBestPracticesSubset
displayName: Recipe nullability best practices
description: Use OpenRewrite internal nullability annotations; drop JetBrains annotations; use `package-info.java` instead.
recipeList:
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: org.jetbrains.annotations.Nullable
newFullyQualifiedTypeName: org.openrewrite.internal.lang.Nullable
- org.openrewrite.java.ChangeType:
oldFullyQualifiedTypeName: jakarta.annotation.Nullable
newFullyQualifiedTypeName: org.openrewrite.internal.lang.Nullable
- org.openrewrite.java.RemoveAnnotation:
annotationPattern: '@org.jetbrains.annotations.NotNull'
- org.openrewrite.java.RemoveAnnotation:
annotationPattern: '@jakarta.annotation.Nonnull'

I'm considering adding that to a root rewrite.yml to make it easier to run these recipes on your local machine. Thoughts?

Ah that's what it was trying to do. The suggestions were not showing that and not applyable.. Yeah sound like a good plan!

@okundzich
Copy link
Member

just talked to Bryce about azuredevops and this came up. We require configuration of baseurl in moderne CLI/agent. curious about this.

* refactor: Azure DevOps refactoring.

* review changes

* review changes

---------

Co-authored-by: Peter Streef <[email protected]>
this.autocrlf = autocrlf;
this.eol = eol;
this.committers = committers;
cloneUrl = parseCloneUrl(origin);
Copy link
Member

Choose a reason for hiding this comment

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

We should probably only calculate this when it is first used, otherwise this slows down deserialization.

pstreef and others added 2 commits August 4, 2024 21:11
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
*/
@Deprecated
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's necessary to deprecate since it is in widespread use. Fine to have a convenience method for organization on CloneUrl

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually I think we should point back to the "old" method that does not take an argument, and deprecate this one which (again) takes the origin (with scheme).

@pstreef
Copy link
Contributor Author

pstreef commented Aug 5, 2024

replaced by #4381

@pstreef pstreef closed this Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants