You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Sometimes weird commits make it into PRs that do something strange (and invalid, codewise) but then revert it in a later commit. I've done this accidentally a few times when rebasing or cleaning up the git history to prep a PR after some experimentation.
We should have a github action that makes sure all commits in a PR at least compile. They don't need to pass all tests or even strictly pass linting, but the following should at least run without error:
git rebase $(git merge-base origin/main HEAD) --exec 'go build ./...'
I was always a bit skeptical about requiring something like this because I'd rather prioritise readability. Sometimes it can be a little difficult to make clean, isolated commits when also trying to keep this requirement. For example, sometimes it looks cleaner to modify a function that requires modifying the function's signature in one commit and then have one or more commits after that go through every other subpackage that calls it and update the function calls to match. However, I'm coming around to thinking this isn't that problematic in the end and it works out fine, and is just as readable, if the signature changes in a single commit along with all the function calls, while the implementation that required it happens after.
A description for this requirement should be added to the contributor guide as well.
The text was updated successfully, but these errors were encountered:
Sometimes weird commits make it into PRs that do something strange (and invalid, codewise) but then revert it in a later commit. I've done this accidentally a few times when rebasing or cleaning up the git history to prep a PR after some experimentation.
We should have a github action that makes sure all commits in a PR at least compile. They don't need to pass all tests or even strictly pass linting, but the following should at least run without error:
I was always a bit skeptical about requiring something like this because I'd rather prioritise readability. Sometimes it can be a little difficult to make clean, isolated commits when also trying to keep this requirement. For example, sometimes it looks cleaner to modify a function that requires modifying the function's signature in one commit and then have one or more commits after that go through every other subpackage that calls it and update the function calls to match. However, I'm coming around to thinking this isn't that problematic in the end and it works out fine, and is just as readable, if the signature changes in a single commit along with all the function calls, while the implementation that required it happens after.
A description for this requirement should be added to the contributor guide as well.
The text was updated successfully, but these errors were encountered: