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

Markdown: should we require one sentence per line? #25

Closed
scottrigby opened this issue Aug 22, 2020 · 10 comments
Closed

Markdown: should we require one sentence per line? #25

scottrigby opened this issue Aug 22, 2020 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed lifecycle/stale

Comments

@scottrigby
Copy link
Member

📊 Maintainers poll: should we require one sentence per line in markdown files?

I initially removed these from the READMEs where they existed, but am rethinking that now. I removed them because they were not consistent. But maybe it's better to make them consistent the other way, by adding them back, and then requiring that moving ahead. https://sembr.org/ makes a compelling case for this, and I'm inclined to agree.

This issue is only about whether maintainers also agree, and want this to be part of this repo style guide. Thoughts?

Also see this related issue about whether to enforce markdownlint in general: #19

For options, see:

@torstenwalter
Copy link
Member

@scottrigby nice references! I like the article linked from denver.org https://rhodesmill.org/brandon/2012/one-sentence-per-line/

I agree that semantic line breaks / one sentence per line makes it easier to review changes.
I have no experience with linters to enforce it.
A good start would be to disable rules which force line breaks after 80 characters when linting markdown.

@gkarthiks
Copy link
Member

@scottrigby I like the semantics breaks. It's also easy on eyes, doesn't need to read a statement twice to make sense of.

Here's my +1 on this.

@monotek
Copy link
Member

monotek commented Aug 23, 2020

Could we add this as a requirement to the ci pipeline / linter maybe?
If so, sounds good to me :)

@scottrigby
Copy link
Member Author

@JoshuaKGoldberg pointed me here JoshuaKGoldberg/sentences-per-line#6 and said it should be doable, the rule is basically a glorified regular expression

@JoshuaKGoldberg
Copy link

JoshuaKGoldberg commented Sep 5, 2020

It would be an honor to have my teeny markdownlint rule used here ☺️ and I would be happy to support as needed!

@torstenwalter
Copy link
Member

torstenwalter commented Sep 5, 2020

It would be an honor to have my teeny markdownlint rule used here and I would be happy to support as needed!

@JoshuaKGoldberg Do you know if it's possible to in include your rule in https://github.com/github/super-linter?

Markdownlint is called here: https://github.com/github/super-linter/blob/c58bab262756890004e5bbb3cf8744ed1e6174c4/lib/linter.sh#L1646

@JoshuaKGoldberg
Copy link

😕 no, it looks like it's not possible to include custom -r/--rules markdownlint rules in super-linter right now. I'll defer to @DavidAnson as to whether there's any interest in allowing those rules to be configured via the config file.

Alternately, maybe super-linter could take an issue to allow configuring those things? I haven't used it enough to know their project ethos very well.

@DavidAnson
Copy link

@JoshuaKGoldberg Maybe add the markdownlint-rule keyword to your package so it shows up here for other people to find: https://www.npmjs.com/search?q=keywords:markdownlint-rule

@JoshuaKGoldberg You may be able to use the markdownlint-rule-helpers package to remove the dependency of your rule on internals of the linter: https://www.npmjs.com/package/markdownlint-rule-helpers

@JoshuaKGoldberg I added a note to the issue above to address one of the concerns therein.

Everyone: The .markdownlint.json configuration file is exclusively for configuring rules, so that doesn't help the scenario above. However, I created an alternate version of the CLI that is entirely configuration-file-based. This scenario where it's not convenient to update the caller of the tool is sort of kind of what I had in mind when I created it. Perhaps that could help: https://github.com/DavidAnson/markdownlint-cli2

Everyone: you may also be interested in this issue with a somewhat more detailed discussion of line breaking: DavidAnson/markdownlint#298

@stale
Copy link

stale bot commented Oct 27, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Any further update will cause the issue/pull request to no longer be considered stale. Thank you for your contributions.

@stale
Copy link

stale bot commented Nov 10, 2020

This issue is being automatically closed due to inactivity.

@stale stale bot closed this as completed Nov 10, 2020
nf-npieros pushed a commit to nf-npieros/helm-charts that referenced this issue Jan 17, 2022
nf-npieros pushed a commit to nf-npieros/helm-charts that referenced this issue Jan 17, 2022
junotx pushed a commit to junotx/prometheus-helm-charts that referenced this issue Oct 12, 2023
…acProxy

[prometheus-blackbox-exporter] Allow to enable kube-rbac-proxy for prometheus-blackbox-exporter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed lifecycle/stale
Projects
None yet
Development

No branches or pull requests

6 participants