Skip to content

Conversation

@romanowski
Copy link
Contributor

@romanowski romanowski commented Jun 14, 2021

fixes #12762

fixes problems with sourcelink tests

@romanowski romanowski force-pushed the scaladoc/fix-sourcelinks branch 2 times, most recently from 41f0805 to 827b301 Compare June 14, 2021 09:29
@romanowski romanowski force-pushed the scaladoc/fix-sourcelinks branch from 827b301 to 1f7f355 Compare June 14, 2021 09:44
The problem was caused by stripping extension incorrectly from files
@romanowski romanowski requested a review from BarkingBad June 14, 2021 11:48
@romanowski romanowski marked this pull request as ready for review June 14, 2021 11:48
@romanowski
Copy link
Contributor Author

romanowski commented Jun 14, 2021

Doc generated using this PR - latest blogpost links correctly

Copy link
Contributor

@BarkingBad BarkingBad left a comment

Choose a reason for hiding this comment

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

Changes seem OK, though I have few questions.

<li>
<h2>
<a href="{{ site.baseurl }}{{ post.url }}">{{ post.title }}</a>
<a href="{{ post.url }}">{{ post.title }}</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

Was site.baseurl empty string?

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, we do not expose such setting

def scalaSrcLink(v: String, s: String) = s"-source-links:$s=github://scala/scala/v$v#src/library"
def dottySrcLink(v: String, s: String) = s"-source-links:$s=github://lampepfl/dotty/$v#library/src"
def dottySrcLink(v: String, s: String) =
sys.env.get("GITHUB_SHA") match {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this environment variables driven logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that we want to link to the source code that has been actually used for this PR, not the 3.0.0 sources. Before we tried to link to e.g. experimental.scala that that not exists in 3.0.0

val toLine = expectedLine + 3
val memberToMatch = member.replace("`", "")
assertTrue(s"Expected to find $memberToMatch at $link at line $line", loc.contains(memberToMatch))
val lineCorrectlyDefined = (expectedLine.toInt until toLine.toInt).exists{ line =>
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand it correctly we look for the member in the range of lines. Was there something failing for the exact line match?

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, in was the case of name that spans in multiple lines like asMatchable: https://github.com/lampepfl/dotty/blob/3.0.0/library/src/scala/compiletime/package.scala#L169-L170

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.

Blog post links are generated incorrectly

3 participants