multi: enforce 80-char line length and add nolint:ll comments
#1189
Conversation
Summary of ChangesHello @ffranr, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant improvement in code consistency and readability by enforcing an 80-character line length limit throughout the codebase. This change is implemented by configuring the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a line length limit of 80 characters and applies the necessary formatting changes across the codebase. The changes are mostly stylistic and look good. I've found a few minor inconsistencies in the formatting of nolint comments and have left suggestions to make them consistent.
ffranr
force-pushed
the
lint-line-length
branch
from
0c0ad20 to
2f602b6
Compare
ellemouton
left a comment
ellemouton
left a comment
There was a problem hiding this comment.
i think maybe we should take this opportunity to use the same custom ll linter that LND uses? that way things work nicely for structured logging
- Replace the default `lll` with a custom `ll` linter, enabling configurable exclusions for specific `S` log lines. - Integrate custom `ll` linter into the build system and `Makefile`. - Include relevant test cases and configuration for `golangci-lint`.
- Replace occurrences of `// nolint:lll` with `// nolint:ll` across files for consistency. - Reformat multiline strings, comments, and function parameters to improve clarity and adhere to style guidelines. - Add `// nolint:ll` comments where necessary to prevent linter warnings.
ffranr
force-pushed
the
lint-line-length
branch
from
2f602b6 to
a0e6312
Compare
ffranr
changed the title
nolint:lll commentsnolint:ll comments
|
I've now added |
ellemouton
left a comment
ellemouton
left a comment
There was a problem hiding this comment.
thank you!! much appreciated 🚀
ViktorT-11
left a comment
ViktorT-11
left a comment
There was a problem hiding this comment.
Thanks a lot for this @ffranr 🔥!!
| chanPoint, err = | ||
| firewalldb.HideChanPointStr( | ||
| ctx, tx, chanPoint, | ||
| ) |
There was a problem hiding this comment.
I don't think we've used this pattern previously in litd, where you'd start the values on the right side of a = char on a new line. I'm ok with introducing it though, but I'm also in slight favour of just using nolint:ll for these types of lines. There's quite of few of these examples throughout the changed files.
I'll let you decide and I'm definitely ok with merging this as is if you think we should use this pattern :)!
There was a problem hiding this comment.
I'm not a big fan of the newline after = style. But I do think that in some cases its better than the alternative. It is used in tap and loop:
https://github.com/lightninglabs/loop/blob/f76c15ca16442b19b296b332ff77fb684729ecfa/staticaddr/withdraw/manager.go#L660-L664
So I think it should be ok for now. Thanks for the review!
.golangci.ymlto enforce a maximum line length of 80 characters.nolint: lllcomments where line wrapping was not applicable.