Automated Code Review Comments
Objective
Provide feedback to GitLab Engineers on violations for best practices using Code Review comments within MRs.
Proposal
Use danger (or something similar) to post code review suggestion when a violation of a suggested practice occurs that can be applied by the author. An example is when match
is used instead of match_array
in specs order dependent failures can occur: !62606 (diffs, comment 603229803).
Discussion
!62606 (diffs, comment 603229803)
I noticed 3 different specs failing for an array-ordering expectation problem today.
😞 I wish people would catch that during reviews as this is a simple thing to see.Could we create review comments on MRs with suggestions that those type of assertions may cause array ordering flaky failures? Bypass the step to make people think about catching it in review and try to nudge them towards it when it would apply.
Right I was thinking some sort of enhancement to Danger (or something completely different) that would intercept changes to specs and when it observes lines changed with expect(projects).to match [accessible_project, accessible_project_2, search_project] it would add a code review comment of expect(projects).to match_array [accessible_project, accessible_project_2, search_project] with a comment explaining the rationale and allow the author/reviewer to resolve it it doesn’t apply.
Yeah that’s a good idea, we could catch match([ and match [ at first