Skip to content

Simplify static site configuration. Sort pages loaded from file system #15099

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

Closed

Conversation

pikinier20
Copy link
Contributor

@pikinier20 pikinier20 commented May 4, 2022

fixes #15079

This PR simplifies our static site configuration by providing whole directories instead of specific files.
I've made a diff between static site before and after these changes. There are no missing pages but few pages got added:

< ./docs/internals/explicit-nulls.html
< ./docs/internals/syntax-3.1.html
< ./docs/reference/changed-features/interpolation-escapes.html
< ./docs/reference/metaprogramming/simple-smp.html
< ./docs/reference/other-new-features/indentation-experimental.html
< ./docs/usage/coverage.html
< ./docs/usage/scaladoc/blog.html

We should decide whether all of these pages should be present in static site and remove unnecessary ones. (We can always retrieve them from Git)

The explicit-nulls file is problematic since we've got two versions of it in internals and reference/experimental and one of these file has annotation that it's moved to docs.scala-lang.org. Tbh, I don't know which information in which file is correct.

@odersky it seems that in syntax-3.1.html there are terminating semicolons present
@michelou Is interpolation-escapes file supposed to be present in documentation?
@odersky are simple-smp and indentation-experimental files supposed to be present in documentation?

@julienrf
Copy link
Contributor

julienrf commented May 4, 2022

Please, before adding more content to https://dotty.epfl.ch, we all need to agree on the purpose of this website, and how it relates to https://docs.scala-lang.org.

@smarter
Copy link
Member

smarter commented May 4, 2022

IMO, ultimately dotty.epfl.ch should just redirect to docs.scala-lang. I don't have an opinion on the details of how we get there.

@smarter smarter assigned julienrf and BarkingBad and unassigned smarter May 4, 2022
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.

I'd like to see this merged so the coverage page is online somewhere at least.

@julienrf
Copy link
Contributor

julienrf commented May 4, 2022

IMO, ultimately dotty.epfl.ch should just redirect to docs.scala-lang. I don't have an opinion on the details of how we get there.

I agree, but the simplest way to get there is to already redirect to docs.scala-lang.org by not adding new content to dotty.epfl.ch and putting the new content directly in docs.scala-lang.org.

I think dotty.epfl.ch could still be useful as a way to access the documentation of nightlies. But I don’t think it should be a source of “new” content (I mean, content that could not be found on docs.scala-lang.org). If we decide to change anything to the current state of dotty.epfl.ch, that should be removing existing pages that have an equivalent in docs.scala-lang.org, not adding new content.

@olhotak
Copy link
Contributor

olhotak commented May 4, 2022

For explicit nulls, internals and reference/experimental are two distinct documents: the former is about the implementation and the latter is documentation for users.

I see the latter is different between dotty.epfl.ch and docs.scala-lang.org. In this case, the dotty one is more up to date than the scala-lang one. I don't understand the difference between dotty and scala-lang or which of the two should be the definitive version.

@julienrf
Copy link
Contributor

julienrf commented May 4, 2022

In my opinion the content of usage/coverage.md should be put in the documentation of sbt-scoverage. The compiler option -coverage-out should be covered in the documentation of the Scala 3 compiler options. The documentation about the internals should go to the dotty internal documentation.

@julienrf
Copy link
Contributor

julienrf commented May 4, 2022

I see the latter is different between dotty.epfl.ch and docs.scala-lang.org. In this case, the dotty one is more up to date than the scala-lang one. I don't understand the difference between dotty and scala-lang or which of the two should be the definitive version.

I hear this as another argument that having documentation in multiple places just brings more confusion. About the issue you mention (inconsistent duplication between docs.scala-lang.org and dotty.epfl.ch), there is ongoing work that should be released very soon (hopefully the last bug is resolved by #15102).

Copy link
Contributor

@julienrf julienrf left a comment

Choose a reason for hiding this comment

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

I believe we should not add more content to dotty.epfl.ch except about new experimental features and new internal documentation.

@julienrf
Copy link
Contributor

julienrf commented May 5, 2022

FYI, I have published an alternative idea of solution in #15110, and a more general plan about the Dotty website here: https://github.com/lampepfl/dotty/projects/11

categoryPath.resolve(relativeFile).toFile
}
files.toList
.filter(_.toPath != indexPage.file.toPath)
.flatMap(file => loadRecursively(file, mappingFunc))
.sorted
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure that sorting pages alphabetically is a good default choice. It rarely matches what we want. See e.g. the Enums section in the reference documentation, which now shows first the advanced “Algebraic Data Types” page before the “Enumerations” introduction page. You can compare here with the current docs here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to request an explicit sidebar specifying the desired content order.

Copy link
Member

@bishabosha bishabosha May 11, 2022

Choose a reason for hiding this comment

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

in the current docs.scala-lang the ordering of the page in a "collection" (directory in this case) is part of the file metadata, i.e. a natural number representing the page index - this seems better to me to avoid having to synchronise with an external sidebar file.

We could also add checks to ensure each page has a valid ordering - and perhaps it could be more flexible, e.g. only require that the ordering value has to increase (allowing for more flexible 'buckets' for when we optionally add extra 'details' pages to an existing document, without having to fix the indices of every other page). e.g. (1,2,100,200,300,301, 302, 400 etc.)

 ---
 layout: index
 title: "Other Changed Features"
 movedTo: https://docs.scala-lang.org/scala3/reference/changed-features.html
+ordering: 0
 ---

 The following pages document the features that have changed in Scala 3, compared to Scala 2.

@odersky
Copy link
Contributor

odersky commented May 11, 2022

Ordering alphabetically is a truly terrible idea. It's akin to ordering chapters in a book alphabetically by their title!

There must be a meta rule for site generation that it should NEVER change the contents except for layout. That should go without saying, really.

@odersky it seems that in syntax-3.1.html there are terminating semicolons present

That should be reverted, like the others were.

@odersky are simple-smp and indentation-experimental files supposed to be present in documentation?

Yes, of course! It's quite shocking that indentation-experimental got dropped, it has to be re-instantiated as soon as possible.

Also, another really bad change is that now the "more-details" sections are all in the top-level index. They are not meant to be there. For instance, erased types - more details is only meant to be linked from erased types.

Overall I am exasperated by all the butchering that went on recently on the docs. The docs have become MUCH WORSE than they were. I put a lot of effort into them, and I do not have the time to constantly have an eye that there is no breakage. So I am sorry but I think we need an audit that where we come up with a precise change list of the docs between (say) a year ago and now, to the last semicolon, and then list the changes and get a sign off from the original authors. It will be a lot of work, but I see no other way since the docs have been really butchered beyond recognition. Every detail is important, and every mistake gives a bad image. And I am afraid a lot of mistakes were made.

@Kordyjan
Copy link
Contributor

@odersky: We understand your concerns.

Updating the look and UX of the documentation, simultaneously making it easy to be kept up to date is a hard and laborious task. @pikinier20 is working hard and putting his heart into this project.

This PR was just the bunch of proposed changes up to review. As with all PRs, it could have some problems to be found. Indeed we have found that making the ordering of pages deterministic results in alphabetical ordering for some cases where it was not intended. This problem will of course be fixed before pushing that to production. Other issues, like missing files and terminating semicolons in the EBNF definitions, were spotted even before the creation of this PR and Filip explicitly asked about them. We can be sure that those changes were never intended to be merged before resolving mentioned issues.

I believe that the review process is working as intended and the documentation is slowly getting better.

@odersky
Copy link
Contributor

odersky commented May 11, 2022

Thanks for explaining! I was not aware about the status, of course one should be able to make PRs to figure things out. But if something's not good it would be important to detect that quickly and prevent a merge. In this case there was a "I want to see this merged" several days before the flag about ordering was raised, so there was a real danger that the change would have gone in.

The problem is how to prevent meaning-destroying changes in the future. Unlike for programs we don't have tests for docs that prevent regressions. So I think the only way to do it, which unfortunately is very laborious, is to go through all the docs and write up a report with the PR itself, about what exactly changed from the perspective of a the reader of the docs. Maybe with screenshots before/after if it is layout, or with textual diffs if it is just text. It would be the responsibility of the person making the PR to create that list. If the change summary is incomplete that would count as a bug in the PR. Then a second person with editorial knowledge can approve or request changes.

The problem is that people with editorial authority do not have the time to go through all the docs spotting changes when they review. That's why we should separate the roles of collecting changes from the roles of approving or rejecting them.

Do you think that could work?

@pikinier20
Copy link
Contributor Author

In terms of having screenshots before/after some change, we've got a CI that uploads the documentation with applied changes to Azure so that we can take a look at it before merging a PR. To make this happen, one needs to name a branch with prefix 'scaladoc/'. I think it solves one of problems.

@odersky
Copy link
Contributor

odersky commented May 11, 2022

@pikinier I agree, a way to preview the changes would be very useful. And then take that and make a summary of the changes. That would help, I think.

@Kordyjan
Copy link
Contributor

I'm 100% for the idea of designating one person with editorial knowledge as docs supervisor and requiring their approval under every pr that is making any changes to the docs. The auto-generated preview is a great asset here.

@pikinier20: Would it be possible to make the action, that is publishing the preview, also post a link to it as a comment under the PR introducing the changes.

@pikinier20
Copy link
Contributor Author

@pikinier20: Would it be possible to make the action, that is publishing the preview, also post a link to it as a comment under the PR introducing the changes.

Yes, it shouldn't be a big problem

@pikinier20
Copy link
Contributor Author

I think this PR can be closed since most of these changes are not going to be merged

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.

Documentation table of contents (and docs.scala-lang) is missing various doc pages
8 participants