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

Fixup Most Of The ScalaDoc Issues #4020

Merged
merged 2 commits into from Dec 17, 2020

Conversation

isomarcte
Copy link
Member

This commit corrects most of the Scaladoc issues.

The Scaladoc issues fell into one or more of the following categories.

  • Reference was not fully qualified.
    • For external links, and for some internal links, Scaladoc wants a fully qualified symbol to link against, even if the given symbol is in scope.
    • For example, [[Kleisli]] -> [[cats.data.Kleisli]].
  • Third party library is not exposing apiURL in the POM, or is doing so incorrectly.
    • This prevents SBT from automatically configuring the URLs for linking into the third party Scaladoc.
    • To solve this issue ScaladocApiMapping was added in project/. This contains manual code to construct the linking,
      using https://javadoc.io if the project does not provide another more specific URL.
  • Overloaded references
    • These are warnings caused by a reference to an overloaded symbol.
    • These were not fixed as part of this commit. I tried for quite some time, but I just couldn't get the disambiguation logic to work.
  • The Scaladoc was just wrong.
    • In this some cases the Scaladoc just didn't link to a real symbol, usually because of typos or change symbol names.

Prior to this commit there were 35 warnings issued for Scaladoc (you need to enable link-warnings to see this), now there are 5 and they are all related to overloaded symbols.

This commit corrects _most_ of the Scaladoc issues.

The Scaladoc issues fell into one or more of the following categories.

* Reference was not fully qualified.
    * For external links, and for _some_ internal links, Scaladoc wants a fully qualified symbol to link against, even if the given symbol is in scope.
    * For example, `[[Kleisli]]` -> `[[cats.data.Kleisli]]`.
* Third party library is not exposing `apiURL` in the POM, or is doing so incorrectly.
    * This prevents SBT from automatically configuring the URLs for linking into the third party Scaladoc.
    * To solve this issue `ScaladocApiMapping` was added in `project/`. This contains manual code to construct the linking,
      using `https://javadoc.io` if the project does not provide another more specific URL.
* Overloaded references
    * These are warnings caused by a reference to an overloaded symbol.
    * These were _not fixed as part of this commit_. I tried for quite some time, but I just couldn't get the disambiguation logic to work.
* The Scaladoc was just wrong.
    * In this some cases the Scaladoc just didn't link to a real symbol, usually because of typos or change symbol names.

Prior to this commit there were 35 warnings issued for Scaladoc (you need to enable `link-warnings` to see this), now there are 5 and they are all related to overloaded symbols.
Copy link
Member

@rossabaker rossabaker left a comment

Choose a reason for hiding this comment

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

Oh, very cool. I bet we could cherry-pick back most of this and keep the branches in better sync. I'm being a bit fanatical about pushing work to series/0.21, because right now it's merging to five(!) branches (dotty, main, cats-effect, and cats-effect-dotty to make sure the two big ports come together. 😓)

@isomarcte
Copy link
Member Author

@rossabaker Yikes, that's a lot of branches to manage.

Do you want me to open separate PRs cherry picking this to each of those or do you want to do that on your end?

@isomarcte
Copy link
Member Author

isomarcte commented Dec 17, 2020

Oh gosh. I meant to open this against series/0.21. I keep forgetting that's not the default target...

Is that what you were asking me to do? @rossabaker

@isomarcte isomarcte changed the base branch from main to series/0.21 December 17, 2020 15:05
@isomarcte
Copy link
Member Author

@rossabaker I'm curious on your thoughts about removing -no-link-warnings from the build.

Warnings are currently configured to be fatal in that scope, and it might be nice to have good viability on when a Scaladoc link is bad, especially since Scaladoc's linking rules are so arcane.

@rossabaker
Copy link
Member

I'm used to cherry-picking commits back to series/0.21. Our default is main, so it can show all the recent activity, but but the vast majority of our PRs start on 0.21 right now. But thanks for resetting!

I think we got so frustrated with the arcane rules we gave up and turned on that flag. But if we've got it cleaned up and can catch new ones, I'm in favor of turning it off and seeing how it goes.

@rossabaker rossabaker merged commit 9ff3f93 into http4s:series/0.21 Dec 17, 2020
@isomarcte isomarcte deleted the fixup-scaladoc branch December 17, 2020 16:24
armanbilge pushed a commit to http4s/http4s-play-json that referenced this pull request May 2, 2022
armanbilge pushed a commit to http4s/http4s-twirl that referenced this pull request May 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants