-
Notifications
You must be signed in to change notification settings - Fork 282
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
Add working "get started" example #4855
Add working "get started" example #4855
Conversation
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, it's a good suggestion.
docs/installation.md
Outdated
version = @STABLE_VERSION@ | ||
runner.dialect = scala3 |
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.
- move this to the very first example in the configuration page
- mark each of these two lines there with a
# mandatory
comment - use scala213; if we are talking about complete newbies, using scala3 for 2.13 is likely to lead to very unhappy outcomes.
docs/installation.md
Outdated
@@ -5,6 +5,22 @@ title: Installation | |||
|
|||
You can use Scalafmt from your editor, build tool or terminal. | |||
|
|||
Create a configuration file called `.scalafmt.conf` at the root of your |
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.
replace this section with a simple
## Configuration
head and below it a link to the configuration page, possibly with a short note stressing importance of this step.
@kitbellew Thank you for the great suggestions. I've changed the PR to reflect these (and will flatten the history once we've got to a happy point). As prompted, we now have a working example at the top of the Configuration documentation, linked from the top of the Installation page. One important note is that I've removed the |
align.preset = more // For pretty alignment. | ||
maxColumn = 1234 |
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.
please put it back, it was the subject of a very long debate. if you absolutely must -- although we thought it obvious -- you could add a comment asking user to substitute a real, less fantastic value.
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 understand that these things are complicated, so for sure I'll put it back now 🙏
I want to stress though that my goal here is to document how to use Scalafmt for someone that doesn't already know, something that previously took a careful read of the documentation. One reason I had originally used a separate location for this example was to document the minimal requirements without needing to get into the weeds. The tendency to cater exclusively to the advanced use case is a common problem in Scala tooling, and I constantly see the negative impact it has on developers new to Scala. While I still think this PR helps as-is, the fact that this introductury example is an invalid configuration is a shame!
Perhaps an alternative would be to add a section above the most popular heading that documents the two required fields. The challenge with this (and why I didn't start with this approach) is that it would mean separating the (mandatory) dialect documenation from the (optional) dialectOverride
setting. Another approach would be to add a "quickstart" on the home page as mentioned above, or as its own separate documentation section.
I'll leave these suggestion as something for you and the team to consider after the fact though, no need to extend this PR converstion here, and thanks again for the great feedback ❤️
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 purposely don't want to mention configuration anywhere else
- people don't read this page and [ab]use discord for help with stuff they could have read here
- perhaps one of the recent llms might help with that, though, i haven't tried
- take a look at git blame for the maxColumn line, and should be able to find a heated discussion regarding standards, style guides etc.
- there's no such thing as a canonical scalafmt configuration, yet this example here is invariably taken as that, with the upheaval that it brings.
- so, 1234 was chosen as a pretty obvious stupid number, so people will quickly realize they need to change it, and the choice is not that hard
feel free to separate the two new mandatory lines from the others with a // here are some examples of optional settings
comment.
a1bb8dc
to
1f2fa5b
Compare
Changes the first example in the configuration example so that it includes the mandatory config options, to better serve as a starting point for customisation. We also link to the Configuration section at the top of the Installation page, since the Installation documentation serves as a "Get started" introduction. Taken together, this should help new Scala engineers get started with Scalafmt.
1f2fa5b
to
5679c47
Compare
I've squashed this to the resulting diff, and updated this branch from |
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 me know if you'd like to add a clarifying comment, or you're done.
align.preset = more // For pretty alignment. | ||
maxColumn = 1234 |
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 purposely don't want to mention configuration anywhere else
- people don't read this page and [ab]use discord for help with stuff they could have read here
- perhaps one of the recent llms might help with that, though, i haven't tried
- take a look at git blame for the maxColumn line, and should be able to find a heated discussion regarding standards, style guides etc.
- there's no such thing as a canonical scalafmt configuration, yet this example here is invariably taken as that, with the upheaval that it brings.
- so, 1234 was chosen as a pretty obvious stupid number, so people will quickly realize they need to change it, and the choice is not that hard
feel free to separate the two new mandatory lines from the others with a // here are some examples of optional settings
comment.
I've added your suggested comment, I do think this helps! Let me know if this looks ok, and thanks again for all your attention here 🙏 |
At the moment, the "Get started" button on the homepage links to the Installation documentation, but doesn't include instructions on setting Scalafmt up for a project. I have observed that this creates significant friction for engineers that are new to Scala.
It's tough to discover that there are two required pieces of configuration, and what the values of these should be. Even on the configuration page (which addresses more complex examples) there's no clear of example of a minimal working config definition that teams can start from. This means Scalafmt is only accessible to engineers that already know how it works, or that have somewhere else to copy a working config file from.
This change adds a note to the top of the Installation documentation, because:
We use the
@STABLE_VERSION@
reference so that these docs will always point to the correct version of Scalafmt. This is especially important because there are two different versions of Scalafmt (scalafmt-core
/sbt-scalafmt
), which is a detail that someone just setting up IDE formatting won't need to understand.I think this proposal addresses the 99% use case of someone just wanting sensible auto-formatting in their IDE and needing to fill in the required pieces of configuration without any customisation. I'm open to the idea that something like this could go on the homepage (at https://scalameta.org/scalafmt/) instead and of course, if this isn't aligned with the documentation's goals then let's chat about that instead 🙏