I honestly ignore the checklist.
> On Apr 8, 2021, at 2:24 PM, Anilkumar Gingade <aging...@vmware.com> wrote: > > It is been some time we have been using a standard check-list for PRs. It > may be time to look back and see if any of them were obsolete; and add new > items based on the PR review experience. > > Current PR check list items: > > 1. Is there a JIRA ticket associated with this PR? Is it referenced in the > commit message? > 2. Has your PR been rebased against the latest commit within the target > branch (typically develop)? > 3. Is your initial contribution a single, squashed commit? > 4. Does gradlew build run cleanly? > 5. Have you written or updated unit tests to verify your changes? > 6. If adding new dependencies to the code, are these dependencies licensed > in a way that is compatible for inclusion under ASF 2.0? > Based on how the PRs are created and with review requirement/process in > place, the check-list from #1 - #5 seems to be obsolete. > > * Ticket numbers are used/referred in PR > * The merging option shows if there is any conflict with base repo > * Unit tests are run on the PR and reviewers are expected to look for > new/existing tests to be added/modified. > > Adding some of the criteria we often miss out during the code changes could > be more valuable to add into the checklist, e.g.: > > * Any serialization changes done; requiring > backward-compatibility/rolling-upgrade testing > * Is there any performance implication with these changes > * Was there any RFC for this PR and needs to be updated (link to the RFC) > > > Please share your thoughts/comments. > > Thanks, > -Anil >
