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

Add linting to Go backend #1226

Closed
k0l11 opened this issue Jan 16, 2023 · 2 comments · Fixed by #1705
Closed

Add linting to Go backend #1226

k0l11 opened this issue Jan 16, 2023 · 2 comments · Fixed by #1705
Labels
area: devops related to the development/build process kind: enhancement New feature or request

Comments

@k0l11
Copy link
Collaborator

k0l11 commented Jan 16, 2023

To make code style consistent and avoid bad practices, we should introduce linting to our Go backend. I think it would be nice to run these both as a pre-commit hook and in CI (GitHub Actions).

VS Code related:

  • It would also be nice if we can make it run on save for people using VS Code as their editor. In order to make this easy to adopt, we should create a .vscode folder with recommended extensions and settings.
  • Since VS Code has some problems related to this settings management, I propose only having a few top level folders in this repository like: frontend (gcsim.app), backend (all Go backend related files) and docs (docs.gcsim.app). After that we can place .vscode inside of backend.
  • In order for .vscode to be picked up, people using VS Code should open the backend folder only. Opening VS Code in a subfolder/higher level folder would make it unable to pick up the .vscode folder. This restriction should be communicated somewhere.

some relevant links:

@k0l11 k0l11 added kind: enhancement New feature or request area: devops related to the development/build process labels Jan 16, 2023
@unleashurgeek
Copy link
Collaborator

It would also be nice if we can make it run on save for people using VS Code as their editor. In order to make this easy to adopt, we should create a .vscode folder with recommended extensions and settings.

I highly agree with this and something I mentioned in the past as part of the UI refactor. I think this is less of an issue for the go side where vscode can work pretty barebones and is more of a problem with the UI side since certain extensions are necessary to make it function.

Since VS Code has some microsoft/vscode#32693 related to this settings management, I propose only having a few top level folders in this repository like: frontend (gcsim.app), backend (all Go backend related files) and docs (docs.gcsim.app). After that we can place .vscode inside of backend.
In order for .vscode to be picked up, people using VS Code should open the backend folder only. Opening VS Code in a subfolder/higher level folder would make it unable to pick up the .vscode folder. This restriction should be communicated somewhere.

This I am a bit more hesitant on due to how different setups might be using our repo (and no hard guarantee that someone is even using vscode). IE: with my current setup I have the repo inside a vscode workspace where it sits adjacent to other projects. Changing how we have to use the repo would require changes to my setup. For the folks that don't use vscode, which has been at least 3 contributors from what I have seen, we'd be also lessening their support.

Not saying that it is necessarily bad to go that route, but personally I'd try as much as we can without it/only do it as a last resort due to the other side effects it can bring.

To make code style consistent and avoid bad practices, we should introduce linting to our Go backend. I think it would be nice to run these both as a pre-commit hook and in CI (GitHub Actions).

IMO I'd want to initial avoid pre-commit hooks. While they can be great when they are working properly, they can also get in the way when you are trying to do a quick fix/something not completely standard and only results in a headache. Because all code is required to go through a PR before it hits mainline, I think making sure everything is done on github CI (with our ability to bypass/ignore if necessary) should be more than enough.

If we change the gh actions before feature-stats-rework is merged, we should probably use that branch's actions as our starting point. They have been refactor there to make it easier to add linting and other verification tools for both builds and PRs.

@k0l11
Copy link
Collaborator Author

k0l11 commented Jan 16, 2023

Both VS Code and pre-commit can be skipped if the person doesn't want to use them (start vs code on top level folder / supply -n/--no-verify to a commit on cli or select the no verify commit variant in your favorite git ui). I think it's ok to add these things, describe them as optional but recommended and show ways to bypass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: devops related to the development/build process kind: enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants