search

scylladb / scylla-bench

43 stars 36 forks source link

Add precommit #54

Closed dkropachev closed 3 years ago

dkropachev commented 3 years ago

Looking to improve code quality

psarna commented 3 years ago

Somebody fluent in golang and its environment should review this one - I have no idea what's happening here, why some new dependencies from github appeared, etc. I'm sure there's a reason for that, but I'm in no position to judge because I never coded a single program in go (:

dkropachev commented 3 years ago

@sitano , Ivan could you please take a look

sitano commented 3 years ago

My opinion is that such tools as style code checking must be part of the GitHub actions / CI/CD / checks before merge, and not be implemented as git pre-commit hooks as it is for example done here:

The level on which those tools must check if the commit satisfy rules must be Git Hub Pull Request. Being implemented as pre-commit hooks it seems to me they will prevent efficient work with git rebase and wip branches.

sitano commented 3 years ago

An example of GitHub CI in PR https://github.com/cockroachdb/cockroach/pull/66246

sitano commented 3 years ago

have to ask about go.mod - where do all those weird version number and hashes come from?

go.mod is part of the standard native golang dependency management system called go mod. it contains versions of all dependencies so you can have reproducible builds.

What motivated you to upgrade go-spew (for example) from v1.1.0 to v1.1.1 - or was this just some sort of automatic process taking the latest version?

In Go it's a usual practice to update packages to the latest version. Not much sense to keep them outdated. But you can choose to update only 1 dependency or all of them. The process of updating / adding something requires you to solve and update dependency constraints in the dep tree and it may require other pckgs to also bump their versions.