chore: upgrade .golangci.yml and workflow to v2 #6924
Conversation
run `golangci-lint fmt` Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
mohammed90
added
under review 🧐
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
|
@WeidiDeng, please take a look. The linter complaints included code lines in the listener management, and those parts of the code scare me. I already introduced and fixed one bug of infinite recursion. I'm afraid I did something else 👀 |
| if !quoted && !(heredoc != heredocClosed || heredocEscaped) && | ||
| if !quoted && (heredoc == heredocClosed && !heredocEscaped) && |
There was a problem hiding this comment.
I'm not sure this one is a valuable change, I think there's a usefulness behind this condition. But maybe it's better to extract this out to a var with a clearer name. I dunno.
There was a problem hiding this comment.
I honestly struggled to read the former form 😅 How would you read it? It'll help me find a suitable variable name. The new form is read in my head as:
If not quoted, and heredoc is closed and not escaped.
| if !(quoted || btQuoted) && !(inHeredoc || heredocEscaped) && | ||
| if (!quoted && !btQuoted) && (!inHeredoc && !heredocEscaped) && |
There was a problem hiding this comment.
This one is okay I think, but both are easy enough to read I think.
|
I'm glad fmt easily fixes the imports order now, very nice addition. We should maybe add that to the contribution doc to mention how to run it |
|
The changes regarding listeners are unreasonable. Those embedded fields are explicitly referred to so it's clear whose methods or fields we are using. Because of the way caddy usage pool is designed, there are many cases of struct embedding. I see the new linter shortens many these types of embedding, and to be fair, I don't like it. Rather, I'd like for the linter to expand some of the embedding to its full form or not care about it at all. |
Noted. I'll figure out which linting rule triggers this and disable it. |
Signed-off-by: Mohammed Al Sahaf <msaa1990@gmail.com>
|
Embeds in several other parts are shortened when it's better to leave them as they're, such as headers and response writers. I think the linter should be run on the unmodified source before this bump. |
|
Changing it to draft until I resolve the review points. It might take me some time. |
I also ran
golangci-lint fmt, which formats all files including tests. It found a couple issues, which is good.