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 static checks #218

Merged
merged 2 commits into from
Jan 15, 2021
Merged

Add static checks #218

merged 2 commits into from
Jan 15, 2021

Conversation

mik-laj
Copy link
Contributor

@mik-laj mik-laj commented Oct 7, 2020

Code quality is important, especially in an open-source project where the code will be maintained for a long time and by multiple people. So I would like to suggest using https://pre-commit.com framework to automate a lot of stuff and assure that our code is nice.

It plays nicely with Github action and allows a plethora of checks (building TOC, license insert, liniting etc) and gives the possibility to build custom ones. For example, here's configuration from Apache Airflow:
https://github.com/apache/airflow/blob/master/.pre-commit-config.yaml

For now I only focused on the basic checks, but in the future, we may add some more Go project-specific checks e.g.golangci-lint or other checks

@mik-laj
Copy link
Contributor Author

mik-laj commented Oct 7, 2020

@tstromberg WDYT?

@TobKed
Copy link

TobKed commented Oct 7, 2020

I have suggestion that for easier review of changes which add linters is to make separate commits for adding linters and parsing project files to distinguish manual and automatic changes.

@mik-laj
Copy link
Contributor Author

mik-laj commented Oct 7, 2020

@TobKed This is not a very big project. Do you think it's worth the extra work to keep these changes separate?

@TobKed
Copy link

TobKed commented Oct 7, 2020

@mik-laj in this case, when it is already done it is probably not worth extra work. However in general, IMHO, it is good practice to keep manual/automatic changes separated 😄

@tstromberg
Copy link
Collaborator

I'm not sure how I feel about the TOC changes yet. - but at this point, let's just get this in and sort out the changes later.

@tstromberg tstromberg merged commit 7bbd00b into google:master Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants