Skip to content

Scaladoc: Add scaladoc binary test #12789

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

Merged
merged 1 commit into from
Jun 22, 2021

Conversation

pikinier20
Copy link
Contributor

Ordered members by name due to indeterministic order of members coming from TASTY.

Copy link
Contributor

@romanowski romanowski left a comment

Choose a reason for hiding this comment

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

It seems that there is some localdevelopment leftover and I am not sure why we need to make some snippet as failing

"-skip-by-id:scala.runtime.MatchCase" \
-project-footer "Copyright (c) 2002-2021, LAMP/EPFL" \
-author -groups -revision master -project-version 3.0.2-RC1-bin-SNAPSHOT \
/Users/fzybala/dotty/out/bootstrap/scaladoc-testcases/scala-3.0.2-RC1-bin-SNAPSHOT-nonbootstrapped/classes 2>&1 || echo "generated testcases project with scripts"
Copy link
Contributor

Choose a reason for hiding this comment

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

/Users/fzybala/dotty looks strange

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, I've missed that

@@ -9,7 +9,7 @@ trait Quotes2[A] {
/**
* SNIPPET(OUTERLINEOFFSET:13,OUTERCOLUMNOFFSET:10,INNERLINEOFFSET:6,INNERCOLUMNOFFSET:6)
* ERROR(LINE:13,COLUMN:12)
* ```scala sc:compile
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 to change that script to fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's because the snippets that I've fixed, had wrong flags and were throwing errors on documentation generation. It caused the process to exit with failure and break the script execution

@romanowski romanowski self-assigned this Jun 14, 2021
@pikinier20 pikinier20 force-pushed the scaladoc-script-tests branch from a2856c9 to 065db77 Compare June 21, 2021 07:16
@pikinier20 pikinier20 requested a review from smarter June 21, 2021 07:41
Comment on lines 1272 to 1274
0
} catch {
case _ : Throwable => 1
Copy link
Member

Choose a reason for hiding this comment

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

Do you actually need to return 0 or 1 here instead of letting it throw an exception if it fails?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tbh it's the way it was when we ran part of tasks using script and the rest using runMain. Integer return type was there for compatibility between these two approaches but now it can be removed

@pikinier20 pikinier20 force-pushed the scaladoc-script-tests branch from 065db77 to 2ae2f64 Compare June 21, 2021 13:58
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Otherwise LGTM

@@ -1265,20 +1265,10 @@ object Build {
s"-source-links:github://lampepfl/dotty/$referenceVersion",
) ++ scalacOptionsDocSettings ++ revision ++ params ++ targets
import _root_.scala.sys.process._
Copy link
Member

Choose a reason for hiding this comment

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

This line is no longer needed.

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.

4 participants