[docs] Refreshes add_ops.md #3939
Conversation
bjacobgordon
force-pushed
the
docs-refreshes-add-ops-md
branch
from
39a20d5 to
e3eb6c6
Compare
|
@marbre I think this'll be an easy one! Could you review it? |
bjacobgordon
force-pushed
the
docs-refreshes-add-ops-md
branch
from
e3eb6c6 to
2135d7e
Compare
docs/add_ops.md
Outdated
|
|
||
| The details of how we do it and helpful commands to help you set up each repo is in [Sungsoon's Shark Getting Started Google Doc](https://docs.google.com/document/d/1H79DwW_wnVzUU81EogwY5ueXgnl-QzKet1p2lnqPar4/edit?pli=1) | ||
|
|
||
| PS: IREE is pronounced Eerie, and hence the ghost icon. | ||
|
|
||
| ## How to begin | ||
|
|
||
| 0. Set up torch-mlir according to the instructions here: <https://github.com/llvm/torch-mlir/blob/main/docs/development.md> | ||
| 1. Set up torch-mlir according to the instructions here: <https://github.com/llvm/torch-mlir/blob/main/docs/development.md> |
There was a problem hiding this comment.
I think it might have been intended to call this step 0?
There was a problem hiding this comment.
I had thought so, too! I did try keeping it, but the linter didn't like it, so I had to look into this to figure out why.
Turns out that "1-based" lists are the standard/default in markdown!
"0-based" lists might be appropriate for:
- documenting code or algorithms where indexing starts at zero
- listing semantic versioning
- matching a particular design or layout requirement
It didn't seem like our situation matched any of those, so I decided to enforce 1-based lists here.
What do you think, do you predict any challenges with this?
There was a problem hiding this comment.
Does this depend on the linter? If I remember correctly, the repo doesn't run a linter on markdown documents on pre-commit or does it? I think if we have a specific linter that always run, in that case it is what it is. Otherwise I would care to much to make the linter happy in that particular case.
There was a problem hiding this comment.
If by "Does this depend on the linter?", you mean
- "Do other linters have the same opinion?": after a brief search, it would appear that this is the de facto linter and there aren't really any others!
- "Is the linter the only thing enforcing this rule?": yes, but I think the only other thing to enforce it is an opinionated engineer that can't help but act on hindsight haha. I believe an opinionated linter here would be beneficial for our documentation efforts since it would help avoid several issues that I've seen as I've been onboarded.
There's no pre-commit markdown linter in place, and I think that might be the very next thing to look at! It appears that it would be based on the very same linter I used while working on this PR.
Thinking out loud:
- there's lots of areas of opportunity within the docs
- some of those areas have to do with formatting best practices
- those best practices are captured/distributed/automated by the linters
- there's really only one markdown linter, and one default config for said linter
- we can use it via IDE extension for now, and look into adding it to pre-commit next
- any IDE-driven style enforcements will reduce the number of unaddressed issues when we add .md linting to pre-commit.
Ergo, keeping the numbering change as is reduces future overhead. 😎
What other questions do you have?
bjacobgordon
force-pushed
the
docs-refreshes-add-ops-md
branch
2 times, most recently
from
ac55868 to
50f1905
Compare
- applied by markdown linter
- applied by markdown linter
- applied by markdown linter
- applied by markdown linter
- applied by markdown linter
- applied by markdown linter
- addresses issues mentioned by markdown linter - avoids less-common 0-based lists since that would require deviation from the default linter config
…locks - addresses issues mentioned by markdown linter
- Each line of detail either: - already existed internally in [Confluence](https://confluence.amd.com/display/SHARK/Turbine+Camp) OR - was severely out of date! - Was beyond the concerns of Torch-MLIR
- was directed to a specific style guide rule rather than the start of the style guide in general
bjacobgordon
force-pushed
the
docs-refreshes-add-ops-md
branch
from
50f1905 to
2bff98d
Compare
Cleaned up this doc while going through Turbine Camp!
Commits are "atomic" as follows:
It'll probably be quicker to review each one individually!