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

docs(code-style-guide): Add comment rules #668

Merged
merged 1 commit into from
Oct 1, 2024
Merged

Conversation

Techassi
Copy link
Member

@Techassi Techassi commented Sep 26, 2024

This adds a bunch of rules regarding Rust comments.

Direct link

@Techassi Techassi self-assigned this Sep 26, 2024
Copy link

netlify bot commented Sep 26, 2024

Deploy Preview for stackable-docs ready!

Name Link
🔨 Latest commit 6fb30e2
🔍 Latest deploy log https://app.netlify.com/sites/stackable-docs/deploys/66f543204132680008499950
😎 Deploy Preview https://deploy-preview-668--stackable-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Techassi Techassi changed the title docs(code-style-guide): Add unit test function name guidance docs(code-style-guide): Add comment rules Sep 26, 2024
Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. Do we need a decision for this? (e.g. I personally prefer 120 chars, but could live with 100)

@Techassi
Copy link
Member Author

Techassi commented Sep 26, 2024

Works for me. Do we need a decision for this? (e.g. I personally prefer 120 chars, but could live with 100)

We can open a decision for this, but tbh, I wouldn't push beyond 100 characters. Rust's std and core library for example also use line-widths between 80 and 100.

Beyond that point, it gets hard to read. It is well known at this point, that you should avoid using long lines of continuous text to easy the process of reading it.

Copy link
Member

@sbernauer sbernauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me. I think we should put it on the decision board, just so that people could veto - but I don't expect any veto

@Techassi
Copy link
Member Author

Moving this into voting. Options are:

  • 👍 Accept as is
  • 👎 Don't accept, post desired changes in comments

@stefanigel
Copy link
Member

"line-widths between 80 and 100" - I was not aware that Rust is so close to FORTRAN :-)

@Techassi
Copy link
Member Author

"line-widths between 80 and 100" - I was not aware that Rust is so close to FORTRAN :-)

I should clarify: Just for the comments, code is usually formatted at 100 and above. But yes, developers like their narrow comments :)

@NickLarsenNZ
Copy link
Member

I just like narrow for reviewing side by side. Otherwise I like 100.

No action needed, just sharing.

@razvan
Copy link
Member

razvan commented Sep 26, 2024

Does the max line length apply to code too?

@Techassi
Copy link
Member Author

Does the max line length apply to code too?

No, we are only talking about comments. Rust code is formatted by rustfmt anyway and it will format with a width of ~100 and above.

@Techassi
Copy link
Member Author

There seems to be no further activity. I will mark this decision as accepted tomorrow and move it to "Done" afterwards.

@Techassi
Copy link
Member Author

Techassi commented Oct 1, 2024

Marking as accepted.

@Techassi Techassi added this pull request to the merge queue Oct 1, 2024
Merged via the queue into main with commit a0a00ba Oct 1, 2024
7 checks passed
@Techassi Techassi deleted the docs/add-comment-rules branch October 1, 2024 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants