-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
KEP template: add guidance for line breaks #5085
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pohly The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
The last three new KEPs that I started to review all didn't use line-wrapping, including one from an experienced Kubernetes contributor. Apparently we need better guidance.
d00bd06
to
1d7c177
Compare
@@ -44,6 +44,10 @@ When editing KEPS, aim for tightly-scoped, single-topic PRs to keep discussions | |||
focused. If you disagree with what is already in a document, open a new PR | |||
with suggested changes. | |||
|
|||
Wrap long lines instead of using one line per paragraph. When updating, change | |||
as few lines as possible. This helps with anchoring review comments. | |||
Semantic line breaks (https://sembr.org/) are useful. |
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.
And to demonstrate semantic line breaks, this last sentence should be its own 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.
Having style guidelines would be great (I completely haven't payed attention to it).
Is there a recommended line width or just semantic line breaks recommendation? Following just semantic, I'd format this entry like this:
Wrap long lines instead of using one line per paragraph.
When updating, change as few lines as possible. This helps with anchoring review comments.
Semantic line breaks (https://sembr.org/) are useful.
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.
You are right, strict sembr would not reflow the first two sentences.
I think just raising awareness should be enough. I don't want to be too pedantic and in particular, don't want reviewers to spend time on enforcing some kind of style guide.
This looks good, but as noted in other PR, let's just hold until after enhancements freeze. /hold |
The last three new KEPs that I started to review all didn't use line-wrapping, including one from an experienced Kubernetes contributor. Apparently we need better guidance.