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

Added version bound check and added slash #643

Merged
merged 2 commits into from
Sep 15, 2023

Conversation

j-mie6
Copy link
Collaborator

@j-mie6 j-mie6 commented Sep 15, 2023

Addresses scala/bug#12867, ensuring that a slash is added for 2.13.12 and above (assuming that this will also be the case for 2.13.13 and 2.14.0 etc).

@mergify mergify bot added the github label Sep 15, 2023
@j-mie6 j-mie6 requested a review from armanbilge September 15, 2023 12:28
@j-mie6
Copy link
Collaborator Author

j-mie6 commented Sep 15, 2023

I've checked this against parsley, as per usual: it correctly links in both 2.13.11 and 2.13.12

Comment on lines +97 to +98
s"${info.browseUrl}/blob/${vh}/€{FILE_PATH}.scala"
else s"${info.browseUrl}/blob/${vh}€{FILE_PATH}.scala"
Copy link
Member

Choose a reason for hiding this comment

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

Dumb question: what if we just always put the slash? Does it work with 2.12, old 2.13s?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wondered about this, then I didn't want to play with 🔥

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Conclusion is, while it does double the slash in the link, this appears to be benign and the link still gets to github ok. However, that feels dodgy to me

@j-mie6
Copy link
Collaborator Author

j-mie6 commented Sep 15, 2023

Applied the desired change (first time using "apply suggestion", that was fun): we now fallback to slash if the patch is not specified.

@armanbilge armanbilge merged commit 5dec904 into typelevel:main Sep 15, 2023
@j-mie6 j-mie6 deleted the fix/scaladoc-link-2.13.12 branch September 15, 2023 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants