-
Notifications
You must be signed in to change notification settings - Fork 45
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
DOCS-685: Use prettier linter #1835
Conversation
Overall readability score: 55.78 (🟢 +1.33)
View detailed metrics🟢 - Shows an increase in readability
Averages:
View metric targets
|
fc0d730
to
2daa84f
Compare
6. Paste the raw fragment contents into the **Raw JSON** config field. | ||
7. Click **Save config**. | ||
8. Now, you can edit the config either in **Raw JSON** mode or in **Builder** mode. | ||
4. Toggle to [**Raw JSON** mode](/try-viam/try-viam-tutorial/#raw-json). |
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.
oh wow
2daa84f
to
b969f09
Compare
31f8f98
to
53fc266
Compare
b298a95
to
fb0710a
Compare
a8f8595
to
c6f1d88
Compare
642bba9
to
4d75b52
Compare
830177f
to
7c0513c
Compare
7c0513c
to
8c6ca95
Compare
c5cfe65
to
12fd734
Compare
12fd734
to
e26665f
Compare
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.
Ok wow! LGTM! Left a question about possible clobbering with the python linter we also just added, and noted a few more instances where we could use <!-- prettier-ignore -->
if desired.
Lots of big diffs makes me a bit nervous, didn't go through all deeply, but everything I spot checked through looks great! Nice work on this!!
"rightm" | ||
], | ||
"left": ["leftm"], | ||
"right": ["rightm"], |
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 know this is JSON, but conceptually this feels like clobbering with the python linter we also just deployed (py linter enforces expansion of some nested components to new lines, for example, counter to what is happening here)
Will prettier ever try to fix python indenting already handled by the py lintiner? Or vice versa? Or are they carefully scoped: (prettier = MD and JSON but skip py code fences || pylintything = py code fences only)?
If so, no worries, should be good!
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.
Prettier skipx python code snippets so all good 😊.
"digital_interrupts": [ | ||
"<your-digital_interrupts-name-1>", | ||
"<your-digital_interrupts-name-2>" | ||
] |
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.
Oh! If the array contains 2+ values, multi-line but if single value = single line (like in docs/components/base/wheeled.md
)?? I like that!
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 think it's if the values can fit on one line without exceeding 80 chars
@@ -97,7 +98,7 @@ If you have another controller that you want to use to control your robot, feel | |||
## Troubleshooting | |||
|
|||
- If you are not able to see a drop-down menu with the name of your controller appear in the **Control** tab, try specifying the `dev_file` attribute to match the exact path to your device. | |||
You can also try setting `auto_reconnect` to `True`. | |||
You can also try setting `auto_reconnect` to `True`. |
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.
Is this right? single item UL element is to be ... indented two whitespace characters? Would have expected -
per other instances.
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 a continuation of the previous item not a new one. So the indentation is to match that item
@@ -102,17 +102,17 @@ Edit and fill in the attributes as applicable. | |||
|
|||
The following attributes are available for a `gps-nmea-rtk-pmtk` movement sensor: | |||
|
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 also want a <!-- prettier-ignore -->
here too?
@@ -102,15 +101,15 @@ Edit and fill in the attributes as applicable. | |||
|
|||
The following attributes are available for a `gps-nmea-rtk-pmtk` movement sensor: | |||
|
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 also want a <!-- prettier-ignore -->
here too?
}, | ||
"ticks_per_rotation": 100 | ||
}, | ||
"depends_on": ["board1", "encoder1"] |
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 odd. Does it have to do that?
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.
Yes it does that to conserve space if the array is short enough to fit on one 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.
My concern is just that this is different from what the app does--I wish we could get it to match the app so that people can seamlessly cross-reference
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.
But I suppose not a common issue since implicit dependencies are the norm, and most places where we still have anything in depends_on could be removed
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.
if there were more here it would split onto multiple lines, so we could do that or change the names if we want to ensure it's on two lines, but let's do that separately.
Co-authored-by: JessamyT <[email protected]>
You can view a rendered version of the docs from this PR at https://docs-test.viam.dev/bb368a369f39e2b2ba210581ee3364f7952f5e24/public |
It would appear prettier for markdown is an all or nothing kind of linter. There is no way to to turn off ANY rules for markdown linting. So whether we want to use it, is up to us:
Advantages:
Disadvantages: