Add Style & Linting - #133
Conversation
Greptile SummaryThe PR replaces the former Beeley style reference with a substantially expanded, mandatory Centrallix coding-style guide and updates related documentation to point to it.
Confidence Score: 5/5The documentation-only PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| AGENTS.md | Directs contributors to the new authoritative style guide and clarifies copyright-update expectations. |
| centrallix-sysdoc/CentrallixCodingStyle.md | Adds the comprehensive project coding-style guide covering formatting, language conventions, error handling, documentation, and copyright notices. |
| centrallix-sysdoc/BeeleyCodingStyle.md | Removes the superseded legacy style guide. |
| centrallix-sysdoc/CodingStandards.md | Marks the new guide as authoritative and updates style and module-prefix references. |
| centrallix-sysdoc/Proposals/ClangFormat.md | Updates the formatter proposal to reference the replacement style guide. |
Reviews (5): Last reviewed commit: "Merge branch 'master' into add-style-lin..." | Re-trigger Greptile
|
The work isn't done, so we can ignore those issues from Greptile for now. They're concerns for later. |
nboard
left a comment
There was a problem hiding this comment.
This looks really promising overall! I may have gotten a bit too much into the weeds on word choice for this early in the project, but hopefully this is helpful overall.
| Styles that still need to be decided and documented: | ||
| - How Markdown files are styled? | ||
| - How Python files are styled. The 26 files in `centrallix-ui-test/tests`. They have no rules today, and several universal rules here do not fit them. | ||
| - How should long expressions or multi-line conditions be broken up? Does the line end with or start with the operator, and how far are continuation lines are indented? |
There was a problem hiding this comment.
TODO
I would like to vote for starting lines with the &&/|| operators. I feel it makes it immediately obvious when reading the line that it belongs to the expression above it, and makes it so deleting/adding lines effect fewer lines
There was a problem hiding this comment.
I agree, but I think I remember cases where this caused issues.
There was a problem hiding this comment.
Yeah, I struggled a lot with this code snippet:
const bool font_changed = false
|| style->FontID != context->CurStyle.FontID
|| style->Color != context->CurStyle.Color
|| realComparePrecision(style->FontSize, context->CurStyle.FontSize, PRT_HTMLFM_FONTSIZE_PRECISION) != 0;I don't really like the false || / true && style I end up using so that we don't have to indent one line differently from every other line.
There was a problem hiding this comment.
I'll make a discussion for this one.
There was a problem hiding this comment.
The parts of this document that overlap with B2CodingStandards.md should probably be trimmed out (though, this can wait until the document is in a more final form to ensure you don't accidentally cut content from both documents)
There was a problem hiding this comment.
Added to TODOs, I'll revisit this later. You can mark this as resolved.
There was a problem hiding this comment.
I'll revisit this after Greg's review ensures that the standard won't change significantly in the near future.
1fa3492 to
72a9f2d
Compare
Improve exception formatting.
Fix some typos.
72a9f2d to
394d049
Compare
…nd the Set Types section.
… the commenting syntax. Fix a typo.
nboard
left a comment
There was a problem hiding this comment.
Looks great overall! Just a couple of suggestions and continued conversations on 2 of the preexisting discussions.
|
The next step is to run a linter with these new rules. However, that merge-conflict all open PRs. |
# Conflicts: # centrallix-sysdoc/BeeleyCodingStyle.md
Add a new, more detailed style guide documenting the new Beeley style in far more detail than the previous document.
TODO
GitHub Relationships
centrallix-libUtilities #130