cds-snc / notification-planning-core

Project planning for GC Notify Core Team
0 stars 0 forks source link

Review and update the reviewers checklist on new PR #304

Open jimleroyer opened 4 months ago

jimleroyer commented 4 months ago

Description

As a code reviewer, I want a checklist of items to verify, So that I can sign off on the changes with confidence and the team' standards.

As a product manager, I want changes to go through a checklist, so that risky changes have reduced impact on production.

WHY are we building?

Not much to build but more to document.

WHAT are we building?

Bring changes to the pull request reviewer's checklist, as discussed in our code review sessions.

VALUE created by our solution

Avoid further incidents with this check list, such as the one that was triggered by the change introduced into production that required full API key format but did not verify if existing users were affected despite a loss in backward compatibility.

Acceptance Criteria

QA Steps

Additional information

jimleroyer commented 4 months ago

The PR for API is up here: https://github.com/cds-snc/notification-api/pull/2122

sastels commented 4 months ago

lots of good feedback, plan to merge in today.

jimleroyer commented 4 months ago

Merged API PR today, and opened & merged one for utils as well.

ben851 commented 3 months ago

Aiming to do one per day!

ben851 commented 3 months ago

Jimmy has to do two today!

ben851 commented 3 months ago

Jimmy has to do three today!

jimleroyer commented 3 months ago

New PRs:

dd-api: https://github.com/cds-snc/notification-document-download-api/pull/160 admin: https://github.com/cds-snc/notification-admin/pull/1772 terraform: https://github.com/cds-snc/notification-terraform/pull/1191 (this one might require some additional changes due to its nature)

jimleroyer commented 3 months ago

manifests: https://github.com/cds-snc/notification-manifests/pull/2458 documentation: https://github.com/cds-snc/notification-documentation/pull/146 lambdas: https://github.com/cds-snc/notification-lambdas/pull/68