Create PR guidelines and simplify checklist #2646
Conversation
pcapriotti
force-pushed
the
pcapriotti/new-checklist
branch
from
b349c05 to
665eb52
Compare
zebot
added
the
ok-to-test
smatting
added
the
not-ok-to-test
.github/pull_request_template.md
Outdated
| - [ ] If a data migration (not schema migration) introduced: measures to be taken by on-premise instance operators. | ||
| - [ ] If public end-points have been changed or added: does nginz need un upgrade? | ||
| - [ ] If internal end-points have been added or changed: which services have to be deployed in a specific order? | ||
| - [ ] Add a new entry in a appropriate subdirectory of `changelog.d` |
There was a problem hiding this comment.
this is also covered by the next point, but i guess it's good for me that i'll have it here because i forget every single time, still. :)
There was a problem hiding this comment.
True, but we decided to add it anyway because it's needed by essentially all PRs. The idea is that the PR author would take relevant checklists from the guidelines document and add them to the PR description, and this one item will basically always be there anyway.
Those files where created very recently. I don't think anyone is going to be looking for them. |
Co-authored-by: Florian Klink <[email protected]>
pcapriotti
force-pushed
the
pcapriotti/new-checklist
branch
from
9fcdafc to
facadd0
Compare
Not sure what I should add to |
|
I just checked: it gets indexed automatically, there is a |
| - [ ] If HTTP endpoint paths have been added or renamed, check [docs/developer/adding-api-endpoints](https://github.com/wireapp/wire-server/blob/develop/docs/legacy/developer/adding-api-endpoints.md) and follow the steps there. | ||
| - [ ] If configuration options have been added or removed, check [docs/developer/adding-config-options](https://github.com/wireapp/wire-server/blob/develop/docs/legacy/developer/adding-config-options.md) and follow the steps there. | ||
| - [ ] If a cassandra schema migration has been added, I ran **`make git-add-cassandra-schema`** to update the cassandra schema documentation. | ||
| - [ ] I swear that if I have changed internal end-points, I do not implicitly rely on deployment ordering (brig needing to be deployed before galley), i.e. I have **not** introduced a new internal endpoint in brig and already make use of if in galley, as I'm aware old deployed galleys would throw 500s until all new version have been rolled out and I do not want a few minutes of downtime. Instead, I have thought how to merge brig an galley codebases. |
There was a problem hiding this comment.
I swear that if I have changed internal end-points ...
This might be worth dicussing, because it has been removed. If we were to keep it in this would mean that we couldn't introduce or change any internal endpoints anymore (in any non-backwards compatible way).
Even such a strategy as making the change in release X and then only start using it in relase X+1 doesn't work, because customers might skip releases when upgrading.
I think there is nothing but to accept 500s during the deployment process for now
/cc @jschaul
There was a problem hiding this comment.
The only real solution would be to start using a versioning system for internal APIs. It seems a bit impractical, but it could be done, potentially.
smatting
removed
the
ok-to-test
This PR simplifies the PR checklist and moves most of the content into a dedicated document containing guidelines to be followed by PR authors.
Checklist