codecov / engineering-team

This is a general repo to use with GH Projects
1 stars 1 forks source link

RCB: Update PR comment with fail reason in different RCB behaviors #1133

Open Adal3n3 opened 7 months ago

Adal3n3 commented 7 months ago

Problem to solve

Description

This task is a part of the work from RCB v2 https://github.com/codecov/engineering-team/issues/1020 It is unclear to customers why the check fails, leading to much confusion. Therefore, on the PR comment, we want to show the transparency: “Your status [fail] because of [reason]” for both patch and project coverage users.

Steps to Reproduce / Current UX
  1. Need a table to list all of the CI check failure scenarios for 1/ reasons based on the RCB behaviors 2/ without RCB which patch coverage only users.
  2. Update the PR comment design with the new conditional check status messages.

Solution

  1. Here's the aligned PR design for displaying all the failed reasons with action for the user. Screenshot 2024-03-01 at 8 56 10 AM

  2. Here's the list of CI failure scenarios and the UI messages. (Reviewed with Gio)

  3. Here is the list of unexpected changes, we can't detect today:

    • Your project check has failed because the coverage report failed to upload.
    • Your project check has failed because you have a time-sensitive tests.
    • Your project check has failed because you are missing coverage reports.
    • Your project check has failed because you have encrypted variables may prevent some execution paths.
    • Your project check has failed because you have a different error handling paths.
  4. Update the unexpected changes to the public document, if needed here

Additional Information

### General Scenarios
- [ ] https://github.com/codecov/engineering-team/issues/1595
- [ ] https://github.com/codecov/engineering-team/issues/1605
- [ ] https://github.com/codecov/engineering-team/issues/1606
- [ ] https://github.com/codecov/engineering-team/issues/1625
- [ ] https://github.com/codecov/engineering-team/issues/1626
- [ ] https://github.com/codecov/engineering-team/issues/1627
### Removed Code Behavior - Adjust Base (Default Config)
- [ ] https://github.com/codecov/engineering-team/issues/1607
### Removed Code Behavior - Removal Only
- [ ] https://github.com/codecov/engineering-team/issues/1608
- [ ] https://github.com/codecov/engineering-team/issues/1609
- [ ] https://github.com/codecov/engineering-team/issues/1610
- [ ] https://github.com/codecov/engineering-team/issues/1611
- [ ] https://github.com/codecov/engineering-team/issues/1612
- [ ] https://github.com/codecov/engineering-team/issues/1615
- [ ] https://github.com/codecov/engineering-team/issues/1616
- [ ] https://github.com/codecov/engineering-team/issues/1617
### Removed Code Behavior - Fully Covered Patch
- [ ] https://github.com/codecov/engineering-team/issues/1618
- [ ] https://github.com/codecov/engineering-team/issues/1619
- [ ] https://github.com/codecov/engineering-team/issues/1620
- [ ] https://github.com/codecov/engineering-team/issues/1621
- [ ] https://github.com/codecov/engineering-team/issues/1622
- [ ] https://github.com/codecov/engineering-team/issues/1623
- [ ] https://github.com/codecov/engineering-team/issues/1624
giovanni-guidini commented 7 months ago

Flowchart with the current possible statuses outcomes and reasons for them. Most likely we will want to change some of those, particular on RCB to make it more clear why the RCB still failed the check. ( I really hope you can download the file and zoom in 😅 )

Based on this we can derive a table of messages we want to add to the PR comment and/or make changes to the current messages in the statuses.

notifications_flow_chart

Adal3n3 commented 5 months ago

Another example: https://twitter.com/edthrn/status/1780476782090604625 Screenshot 2024-04-17 at 6 03 48 PM

Adal3n3 commented 5 months ago

@trent-codecov @aj-codecov Can we prioritize the failed check messaging caused by upload because upload is a part of the onboarding/signup flow.

These issues are upload related:

  1. https://github.com/codecov/engineering-team/issues/1625 (assume we have more patch coverage users)
  2. https://github.com/codecov/engineering-team/issues/1595
  3. https://github.com/codecov/engineering-team/issues/1606
  4. https://github.com/codecov/engineering-team/issues/1609
  5. https://github.com/codecov/engineering-team/issues/1611
  6. https://github.com/codecov/engineering-team/issues/1622
thomasrockhu-codecov commented 2 months ago

Noting that the Q2 items have been completed here: https://github.com/codecov/engineering-team/issues/2094, removing from the Q2 milestone until work is scoped and prioritized

Adal3n3 commented 1 month ago

Making a note here: as discussed during the meeting on 8/12, the next step is for the PM team group and prioritize for us. @rohan-at-sentry

rohan-at-sentry commented 1 month ago

Grouped the items by config type.

My guidance is to tackle General Scenarios first (as it has the broadest surface) and follow with adjust base (as it's the new default).

We can be opportunistic about adding support for other scenarios after that