-
Notifications
You must be signed in to change notification settings - Fork 186
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
Migrate website to docusaurus #808
Conversation
This commit migrates the site to use docusaurus, which has the following benefits: - around 10x faster preview while typing documentation. Changes usually render within ~500ms with automatic browser reload after file save compared to ~10-20 second latency in the previous setup. - splashy landing page
Codecov Report
@@ Coverage Diff @@
## master #808 +/- ##
==========================================
+ Coverage 71.58% 72.55% +0.96%
==========================================
Files 154 158 +4
Lines 3801 3869 +68
Branches 336 331 -5
==========================================
+ Hits 2721 2807 +86
+ Misses 1080 1062 -18
Continue to review full report at Codecov.
|
build.sbt
Outdated
core | ||
libraryDependencies ++= List( | ||
"com.geirsson" %% "metaconfig-docs" % metaconfigV, | ||
"com.geirsson" % "mdoc" % "0.4.0" cross CrossVersion.full, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you need the explicit mdoc
dependency even when using the DocusaursPlugin
?
docs/rule-authors/setup.md
Outdated
|
||
- Assert that a {% glossary_ref LintMessage %} is expected at a particular line by suffixing the line with the comment `// assert: <LintCategory>`. For an example, see the NoInfer test suite: | ||
- Assert that a {% glossary_ref LintMessage %} is expected at a particular line |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These refs are now unhandled.
@@ -1,15 +1,12 @@ | |||
--- | |||
layout: docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it doesn't harm, but you don't need layout
anymore md files
code: Input, | ||
reporter: Reporter | ||
): String = { | ||
scalafix.website.allRulesTable(reporter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline, I would suggest to make this a variable, since it doesn't depend on the code block content (which is, in fact, empty)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses the reporter to warn about missing documentation so I prefer to keep it as a modifier
organizationName: "scalacenter", | ||
|
||
algolia: { | ||
apiKey: "???", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you may want to remove the algolia
section until you have a valid API, so that the search bar doesn't appear
); | ||
|
||
const Features = props => { | ||
const features = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
img
should have alt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for reference, the props is called imageAlt
(it's passed down to GridBlock
)
"rule-authors/glossary" | ||
] | ||
}, | ||
"rules": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we generate this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can, but it's simplest to keep it manual for now. Ideally, I want docs.Main
to take care of generating data like this and also start the live reload server.
{ | ||
title: "Extensible", | ||
content: "Extend Scalafix with custom rewrite and linting rules", | ||
image: imgUrl("extensible.png"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer we avoid using image for this feature. We cannot copy paste, it's not super accessible. I can do a followup PR for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's improve the landing page in a followup PR
| Scalafix Version | Scalameta(SemanticDB) Version | Scala Version | | ||
| ---------------- | ----------------------------- | ----------------------- | | ||
| 0.5.10 | 2.1.7 | 2.11.12 / 2.12.4 | | ||
| @VERSION@ | @SCALAMETA@ | @SCALA211@ / @SCALA212@ | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pain point number one. I would prefer we release a Scalameta version for each Scala Version. I think we discussed this offline and you said it would be to much work. We should make it really clear here what is supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In hindsight, we should have forward ported semanticdb 2.1.7 to 2.12.6 once it came out. This was a special çase however, not even Scala.js forward ports outdated releases for new Scala compiler versions. I did not anticipate it would take this long to get Scalameta v4.0 out and I agree it's caused pain point for users.
As for publishing SemanticDB for 2.12.4 and older versions I'm not convinced it's worth the cost and I haven't seen many requests for this.
|
||
```scala | ||
// project/plugins.sbt | ||
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "@VERSION@") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is incorrect. Right now I see: 0.5.10 + Scala 2.12.6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because 0.5.10 is still the "stable version" until at least v0.6.0-RC1 is released.
docs/users/installation.md
Outdated
"-Ywarn-unused-import" // required by `RemoveUnusedImports` rule | ||
) | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
split code block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
docs/users/installation.md
Outdated
|
||
## Command line | ||
|
||
The recommended way to install the scalafix command-line interface is with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove recommended, since it's the only sane way to install the cli.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, previously there were several alternatives to install the cli (homebrew, wget). Coursier only is simpler.
docs/users/installation.md
Outdated
## Maven | ||
|
||
It is possible to use scalafix with scala-maven-plugin but it requires a custom | ||
setup since there is no scalafix specific Maven plugin. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Help wanted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Also improved the Maven section a bit more.
If using the sbt plugin | ||
|
||
```scala | ||
// project/plugins.sbt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can get this version with a small javascript snippet: scalacenter/bloop#374
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, we can do that in a separate PR
docs/users/installation.md
Outdated
|
||
### --help | ||
|
||
```scala mdoc:--help |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--help is not formatted properly since it's a scala code block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed offline, I would make this a variable instead of a code block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MasseGuillaume the rendered block will not have the scala
language. mdoc should support any language so it's possible to either skip the language prefix or do sh mdoc
. I opened https://github.com/olafurpg/mdoc/issues/57
@gabro If we make --help
a site variable then positions will be messed up for the rest of the document since we search/replace site variables before parsing the markdown document with flexmark. Site variables should be single-line only. Maybe we can come up with a better solution, but until then I think having a custom modifier is best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @gabro and @MasseGuillaume for the review!
| Scalafix Version | Scalameta(SemanticDB) Version | Scala Version | | ||
| ---------------- | ----------------------------- | ----------------------- | | ||
| 0.5.10 | 2.1.7 | 2.11.12 / 2.12.4 | | ||
| @VERSION@ | @SCALAMETA@ | @SCALA211@ / @SCALA212@ | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In hindsight, we should have forward ported semanticdb 2.1.7 to 2.12.6 once it came out. This was a special çase however, not even Scala.js forward ports outdated releases for new Scala compiler versions. I did not anticipate it would take this long to get Scalameta v4.0 out and I agree it's caused pain point for users.
As for publishing SemanticDB for 2.12.4 and older versions I'm not convinced it's worth the cost and I haven't seen many requests for this.
|
||
```scala | ||
// project/plugins.sbt | ||
addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "@VERSION@") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's because 0.5.10 is still the "stable version" until at least v0.6.0-RC1 is released.
docs/users/installation.md
Outdated
"-Ywarn-unused-import" // required by `RemoveUnusedImports` rule | ||
) | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
docs/users/installation.md
Outdated
|
||
- The `semanticdb-scalac` compiler plugin produces | ||
[SemanticDB](https://github.com/scalameta/scalameta/blob/master/semanticdb/semanticdb3/semanticdb3.md) | ||
files in the target directory `META-INF/semanticdb/`. These files are used by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
docs/users/installation.md
Outdated
repository. | ||
|
||
```scala | ||
git clone https://github.com/olafurpg/scalafix-sbt-example |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, the repo is already under scalacenter/
, was a bad link.
I agree confusing to advertise two different examples. The front page links to the template for rule authors while this is an example for end users. It's best to remove the reference to scalafix.g8
on the landing page and use a better image for "Extensible". It's not possible to have git clone workflow for rule authors since filenames dependent on the project name.
If using the sbt plugin | ||
|
||
```scala | ||
// project/plugins.sbt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, we can do that in a separate PR
code: Input, | ||
reporter: Reporter | ||
): String = { | ||
scalafix.website.allRulesTable(reporter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This uses the reporter to warn about missing documentation so I prefer to keep it as a modifier
s"Missing documentation for rule ${rule.name.value} in path $docPath" | ||
) | ||
if (!Files.isRegularFile(docPath.toNIO)) { | ||
reporter.warning(s"Missing $docPath") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc/ @gabro
{ | ||
title: "Extensible", | ||
content: "Extend Scalafix with custom rewrite and linting rules", | ||
image: imgUrl("extensible.png"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's improve the landing page in a followup PR
"rule-authors/glossary" | ||
] | ||
}, | ||
"rules": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can, but it's simplest to keep it manual for now. Ideally, I want docs.Main
to take care of generating data like this and also start the live reload server.
Merging to unblock a PR updating the v1 API. We can iterate on the contents and structure separately, nothing goes live here. |
This commit migrates the site to use docusaurus, which has the following
benefits:
render within ~500ms with automatic browser reload after file save
compared to ~10-20 second latency in the previous setup.
Big thanks to @gabro for setting up the first Docusaurus website for Metals https://scalameta.org/metals/