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

Configure documentation URL for StringJoin check #331

Merged
merged 3 commits into from
Nov 3, 2022

Conversation

gtoison
Copy link
Contributor

@gtoison gtoison commented Nov 3, 2022

Thanks a lot for providing error-prone-support! I'm using it inside SonarQube and the links to the doc are useful.

The link for the StringJoin bug checker should be https://error-prone.picnic.tech/bugpatterns/StringJoin/

@gtoison
Copy link
Contributor Author

gtoison commented Nov 3, 2022

I've removed the unused static import, that caused the build to fail

Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Hi @gtoison

Thanks a lot for opening the PR and congrats on being the first external contributor 😄!

Glad to hear that you like Error Prone Support!

@@ -40,7 +40,7 @@
@AutoService(BugChecker.class)
@BugPattern(
summary = "Prefer `String#join` over `String#format`",
linkType = NONE,
link = BUG_PATTERNS_BASE_URL + "StringJoin",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
link = BUG_PATTERNS_BASE_URL + "StringJoin",
link = BUG_PATTERNS_BASE_URL + "StringJoin",
linkType = CUSTOM,

We should define the linkType as well, otherwise the URL won't be shown as the default is LinkType.AUTOGENERATED.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this should be better now, thanks!

@rickie rickie added the documentation A documentation update label Nov 3, 2022
@rickie rickie added this to the 0.6.0 milestone Nov 3, 2022
@Stephan202
Copy link
Member

Nice catch @gtoison! (Note to self: find time to continue working on this branch.)

Copy link
Member

@Stephan202 Stephan202 left a comment

Choose a reason for hiding this comment

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

Tnx!

Suggested commit message:

Configure documentation URL for `StringJoin` check (#331)

Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Changes LGTM!

W.r.t suggested commit message, I'd omit the word proper as no link is currently shown @Stephan202.
(Made that change)

@rickie rickie changed the title Added the missing link on the StringJoin bug checker Configure documentation URL for StringJoin check Nov 3, 2022
@rickie rickie merged commit d5c1c85 into PicnicSupermarket:master Nov 3, 2022
@gtoison
Copy link
Contributor Author

gtoison commented Nov 3, 2022

Hi @gtoison

Thanks a lot for opening the PR and congrats on being the first external contributor 😄!

Glad to hear that you like Error Prone Support!

It's also commit number 666, wonderful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation A documentation update
Development

Successfully merging this pull request may close these issues.

None yet

3 participants