chore: improve Makefile and add PR checklist #334
Conversation
Plus regenerated files which had been missed in previous PRs.
rrama
changed the title
| @@ -0,0 +1,9 @@ | |||
| ### Description | |||
There was a problem hiding this comment.
Praise: Nice, long overdue!
|
|
||
| - [ ] Tests added and all succeed (`make test`) | ||
| - [ ] Regenerated mocks, etc. (`make generate`) | ||
| - [ ] Linted (`make lint`) |
There was a problem hiding this comment.
Suggestion: let's add a point to integrate this in the CLI to ensure early that the changes didn't break anything.
There was a problem hiding this comment.
Something like this?:
- Test your changes integrated into the CLI
- To test locally uncomment the relevant
replaceline in cliv2/go.mod
- To test locally uncomment the relevant
Or were you thinking they should create a draft PR ready? So adding "If required, create a draft PR on the CLI repo."
There was a problem hiding this comment.
Since we had past occasions where gaf changes broke the CLI and its Acceptance tests, I would say open a PR to test and after merging in gaf ensure to merge in the CLI as well. otherwise we will see multiple changes coming to the CLI at once just because they were never integrated there before making it difficult to debug.
There was a problem hiding this comment.
"open a PR to test", I'm not sure I follow how opening a PR would test it, unless you are suggesting that the person updates the go.mod in the CLI PR to point to their GAF PR's commit?
There was a problem hiding this comment.
Hi @PeterSchafer, I've given the wording an attempt, let me know what you think.
Plus regenerated files which had been missed in previous PRs.
For reference, here is the PR checklist for the LS: https://github.com/snyk/snyk-ls/blob/main/.github/pull_request_template.md