Skip to content

Fix Scaladoc overloaded method link to Duration companion object#4886

Merged
SethTisue merged 1 commit intoscala:2.11.xfrom
janekdb:2.11.x-scaladoc-Duration-links
Jan 14, 2016
Merged

Fix Scaladoc overloaded method link to Duration companion object#4886
SethTisue merged 1 commit intoscala:2.11.xfrom
janekdb:2.11.x-scaladoc-Duration-links

Conversation

@janekdb
Copy link
Member

@janekdb janekdb commented Dec 18, 2015

The links were being skipped with a warning before this commit.

The key change was to remove the result type and add an asterisk.

@scala-jenkins scala-jenkins added this to the 2.11.8 milestone Dec 18, 2015
@janekdb janekdb force-pushed the 2.11.x-scaladoc-Duration-links branch from 15b5b85 to f0b54f1 Compare December 18, 2015 22:34
@janekdb
Copy link
Member Author

janekdb commented Dec 18, 2015

Fixing this required some experimentation which I have noted here for the benefit or amusement of future wanderers.

Test Link Outcome
Test 1 [[Duration.fromNanos Test 1]] Links to wrong overload
Test 1.1 [[Duration$.fromNanos(Double) Test 1.1]] warning
Test 1.2 [[Duration$.fromNanos(Double)* Test 1.2]] warning
Test 1.3 [[Duration$.fromNanos(Dou Test 1.3]] warning
Test 1.4 [[Duration$.fromNanos(Dou* Test 1.4]] warning
Test 2 [[Duration$.fromNanos(Double):Duration* Test 2]] warning
Test 4 [[Duration$.fromNanos(nanos:Double):Duration* Test 4]] warning
Test 5 [[Duration$.fromNanos(nanos:Dou* Test 5]] ✅ Works
Test 6 [[Duration$.fromNanos(nanos:Double* Test 6]] ✅ Works
Test 7 [[Duration$.fromNanos(nanos:Double)* Test 7]] ✅ Works
Test 7.1 [[Duration$.fromNanos(nanos:Double)* Duration]] ✅ Works
Test 7.2 [[Duration$.fromNanos(nanos:Double)* Duration.fromNanos]] ✅ Works
Test 7.3 [[Duration$.fromNanos(nanos:Double)* Duration.fromNanos(]] ✅ Works
Test 7.4 [[Duration$.fromNanos(nanos:Double)* Duration.fromNanos(Double]] ✅ Works
Test 7.5 [[Duration$.fromNanos(nanos:Double)* Duration.fromNanos(Double)]] ✅ Works
Test 8 [[Duration$.fromNanos(nanos:Double):* Test 8]] ✅ Works
Test 9 [[Duration$.fromNanos(nanos:Double):D* Test 9]] warning
Test 9.1 [[Duration$.fromNanos(nanos:Double):Dur* Test 9.1]] warning
Test 10 [[Duration$.fromNanos(nanos:Double):Duration* Test 10]] warning

The outcome of Test 3 is forever lost in the swirling chaos of /dev/null.

Going by this post the tests that warn and include the result type could be fixed by using the fully qualified name of the result type. I see no point in including the result type when distinguishing overloads.

http://stackoverflow.com/questions/23181672/cannot-create-scaladoc-link-for-overloaded-method

Test 1 is interesting. When more than one target is identified the link is made but it could be to the wrong method. Is a best effort approach really helping the author? Failing hard would help when a new overload is added but the docs have been forgotten.

@janekdb
Copy link
Member Author

janekdb commented Dec 18, 2015

Review by @som-snytt, @heathermiller, @dickwall .

@janekdb janekdb force-pushed the 2.11.x-scaladoc-Duration-links branch from f0b54f1 to 25a22ae Compare December 19, 2015 21:50
The links were being skipped with a warning before this commit.

The key change was to remove the result type and add an asterisk.
@janekdb janekdb force-pushed the 2.11.x-scaladoc-Duration-links branch from 25a22ae to 17450e7 Compare January 4, 2016 20:44
@SethTisue
Copy link
Member

/nothingtoseehere

(integrate-ide failure is unrelated to the PR)

@SethTisue
Copy link
Member

@VladUreche maybe you could review?

@som-snytt
Copy link
Contributor

I was scared off by the "works and warnings" table. And maybe by "remove the result type and add an asterisk."

@SethTisue
Copy link
Member

imagine if Scabot would just diff the generated HTML for us poor reviewers...

@janekdb
Copy link
Member Author

janekdb commented Jan 13, 2016

@som-snytt When I was back there, in that half remembered classroom, I was always told to show my working...

@VladUreche
Copy link
Contributor

LGTM, thanks @janekdb! Having a tutorial for links would be greatly beneficial, based on your experiments. Would you be interested in preparing one for docs.scala-lang.org? I can help out with comments and insights.

The outcome of Test 3 is forever lost in the swirling chaos of /dev/null.

What is Test 3? Can you file a bug?

Regarding your latest comment, crashing when multiple overloads are found, you can aways switch from a warning to an error by changing these lines.

@janekdb
Copy link
Member Author

janekdb commented Jan 14, 2016

@VladUreche, please disregard Test 3. It was a test I did and promptly mislaid the outcome. It's not important.

Good idea regarding a tutorial. I've added it to my list, sitting somewhere down from the top but definitely a contender for some attention in Q1!

@VladUreche
Copy link
Contributor

Good idea regarding a tutorial. I've added it to my list, sitting somewhere down from the top but definitely a contender for some attention in Q1!

Awesome, thanks @janekdb! I have some older write-up on links and diagrams which never made it anywhere unfortunately. If you want, I can look it up and send it to you.

SethTisue added a commit that referenced this pull request Jan 14, 2016
Fix Scaladoc overloaded method link to Duration companion object
@SethTisue SethTisue merged commit f6bd712 into scala:2.11.x Jan 14, 2016
@SethTisue
Copy link
Member

thanks Janek, appreciated as always!

@janekdb
Copy link
Member Author

janekdb commented Jan 14, 2016

@VladUreche, instead of direct comms to me please add links as comments on scala/scala-lang#394.

Thanks!

@janekdb janekdb deleted the 2.11.x-scaladoc-Duration-links branch January 14, 2016 15:48
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.

5 participants