ci: add golangci-lint

[dylanhitt]

Any interest in adding staticcheck here?

I've been adding linters to most my repos today and thought I'd hit this one.

I'll fix the staticcheck issues once I hear back.

Have a good weekend. Feel free to close if there isn't any interest.

After discussion this PR ended up adding golangci-lint rather than staticcheck

[EwenQuim]

Good ideas !

I suggest to look at the Makefile and

• directly use make ci-full command so we have the same checks locally and in the CI
• Copy the Makefile steps in the CI. It would create 2 different ways to check things (inconsistent) but is probably better for performance and we would be able to have the tests decompositionore clearly.

What do you think about it ?

[EwenQuim]

Please keep this one, I want to make sure main is OK at any moment!

[dylanhitt]

Same behavior as before.

see: https://github.com/commander-cli/cmd/blob/main/.github/workflows/lint.yaml#L3 for a reference. We can keep it explicit if you want.

[EwenQuim]

This isn't the exact same setup than what's proposed here

[dylanhitt]

Sure. Looks like the makefile could use some work here as well.

So a little look here.

steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-go@v3
      - name: golangci-lint
        uses: golangci/golangci-lint-action@v4

Or

steps:
      - uses: actions/checkout@v3
      - uses: actions/setup-go@v3
      - run: make golangci-lint

My two cents is we decompose with actions and we use the makefile for local testing. Yes, syncing the two will be a burden, but we can mitigate that by using these tools file based config when possible. To me CI is the real truth local ci is optional.

I personally use make to automate local tasks and prefer making the ci as concise as possible using the ci tools native commands.

I think I'm gonna go with using native github actions unless you strongly object. I'll create another PR with some work the Makefile as well.

Edit: turns on golang-ci lint simplifies this greatly. I will try to provide make targets that will compliment fixing issues that golangci-lint catches.

[dylanhitt]

Too add for tools that use the native go toolchain. I will more than likely elect to still use the make commands.

And I'll get something using golangci-lint starting next week and honestly. I don't think we'd have a large syncing issue if we move everything to golangci-lint anyway.

[dylanhitt]

Okay. I think I landed on something that works well. I chose to leverage golangci-lint heavily. If we find it too slow in the future. (I mean we're under a minute now so probably not). We can decompose the commands to individual jobs. If this looks good I'll fix the few issues.

Note: I elected to raise the gofumpt check to have -extra. I think moving forward we should focus on linting through golangci-lint and providing targets in the Makefile to auto resolve them.

Another developer tool we can add the ability to opt in to pre-commit hooks for common mistakes, but that is out of the scope of the goal here. Also I will be editing the PR title to reflect this change.

[EwenQuim]

This isn't the exact same setup than what's proposed here

[dylanhitt]

Looking through the golangci-lint issues. It seems we only have two options to deal with the "unescaped html". Well 3 maybe 4

1. ignore it with a comment annotation
2. change our api around (playing with this it seems to just feel bad) unless you see another pattern
3. Do what the maintainers of gomarkdown suggest

We don't protect against malicious content. When dealing with user-provided markdown, run renderer HTML through HTML sanitizer such as Bluemonday.
We don't protect against malicious content. When dealing with user-provided markdown, run renderer HTML through HTML sanitizer such as Bluemonday.

4. Or four i guess. It's put onto the caller.

[EwenQuim]

Right now:

There are 100 options for markdown. Fuego provide one basic method for ease of use / rapid prototyping. Let's just ignore by commenting the warning, and warn the user that it might be a dangerous method in the docs. The user will create its own custom markdown parser if needed anyway.

---

For the future (I could do this work in a separate PR):

Cf Fuego Extra #118

[EwenQuim]

Thank you for all this, this is a really important PR, good job 👍🏼

[dylanhitt]

@EwenQuim I believe this is officially done.

[EwenQuim]

All green, that's wonderful