Skip to content
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

enable markdown linter #1210

Closed
p0mvn opened this issue Apr 6, 2022 · 5 comments · Fixed by #1478
Closed

enable markdown linter #1210

p0mvn opened this issue Apr 6, 2022 · 5 comments · Fixed by #1478
Assignees
Labels

Comments

@p0mvn
Copy link
Member

p0mvn commented Apr 6, 2022

Context

nit: we should have a markdown liner enabled


* [#1203](https://github.com/osmosis-labs/osmosis/pull/1203) cleanup Makefile and ci workflows

Originally posted by @alexanderbez in #1203 (comment)

Acceptance Criteria

  • markdown linter is enabled and runs on all READMEs
@p0mvn p0mvn moved this to 🔍 Needs Review in Osmosis Chain Development Apr 6, 2022
@p0mvn p0mvn self-assigned this Apr 6, 2022
@p0mvn p0mvn added the frog🐸 label Apr 6, 2022
@alexanderbez
Copy link
Contributor

alexanderbez commented Apr 6, 2022

For reference, see what I did for Umee:

@ValarDragon
Copy link
Member

ValarDragon commented Apr 6, 2022

Here was the old issue I had lol: #88

If people want to retry it, I'm happy to do so. But its the sort of thing I don't think we should feel too attached to / isn't really that helpful.

I think the only ones I found helpful before were links. I found the newlines after header, and line length ones really cumbersome to fight against personally.

@alexanderbez
Copy link
Contributor

The linter will help us have consistency in our markdown, which IMO, is one of the most important things in any codebase, both docs & code wise.

@p0mvn p0mvn moved this from 🔍 Needs Review to 🕒 Todo in Osmosis Chain Development Apr 20, 2022
@p0mvn
Copy link
Member Author

p0mvn commented Apr 21, 2022

Drafted out a PR with this in #1317 to see how this works

Tried running the linter and there were hundreds, maybe thousands of issues. Unfortunately, the linter doesn't auto-fix them all. After auto-fix, it still seems like a few hours worth of work to fix them all manually.

One thing I also noted is that it would require further customization. For example, it complains about having headings with the same name in CHANGELOG

What is the consensus on this? Should I keep working? I would like to make sure that the team wants this before I spend time

Personally, I'm in support of encouraging consistency across the codebase but not sure if it's worth the time effort

@ValarDragon
Copy link
Member

ValarDragon commented Apr 22, 2022

The only markdown lints I've found useful in the past limiting line lengths to 120 characters, and reminders for correct link syntax. I've otherwise found every other one counter-helpful.

Taking unnecesasrry time review/iteration time for things like newline after heading, only using - for lists, and not *, etc. Things that at least to me feel arbitrary. The justification for both of those as far as I'm aware is support with legacy markdown renderers, which I haven't observed these issues with.

I'm happy to adapt if other people want to use it, I'm generally not enthused about them though

Repository owner moved this from Todo 🕒 to Done ✅ in Osmosis Chain Development May 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
3 participants