Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CI] Changes to PR communication #79476

Open
tylersmalley opened this issue Oct 5, 2020 · 6 comments
Open

[CI] Changes to PR communication #79476

tylersmalley opened this issue Oct 5, 2020 · 6 comments
Labels
discuss Team:Operations Team label for Operations Team

Comments

@tylersmalley
Copy link
Contributor

There is currently two-way communication with CI on a PR using comments. While this works, it's not ideal. It's common to have dozens of individuals subscribed, resulting in noise caused by the numerous comments posted. This noise makes Github Notification inbox less efficient as most notifications are not actionable for those subscribed. Those using email notifications minimize this noise by creating complex rules, but it's a less than ideal solution.

Examples of these comments include:

  • CI posting the build status
  • @elasticmachine merge upstream to have CI update the branch
  • jenkins test this to have CI re-run tests on the branch
  • cla/trigger to have the CLI check re-triggered

There are a few alternatives I see to comments.

Slack notifications

When we first introduced posting a comment when CI completed, it was to decrease the feedback loop. Since these notifications are not required, but more of an enhancement to the process, it seems that Slack would be the right solution. We have the mappings for employees Github handles to Slack handles we can utilize.

Dedicated PR status page

While we currently have Github status checks that provide log output for a job, it's only beneficial when a job fails. It would be helpful to have a page available that we control to provide useful information and controls. This page would include a log of past CI runs, current metrics, and controls to update or run-run CI.

PR description

We currently utilize the PR description for issues related to failures on a tracked branch. We could use the PR description to link the dedicated PR status page and possibly information on the last CI run. We have to be careful with these updates as Github does not prevent against overwriting the description if another user updates the description while it's out-of-date.

@tylersmalley tylersmalley added discuss Team:Operations Team label for Operations Team labels Oct 5, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@joshdover
Copy link
Member

Personally, I prefer to have as much info in the GH UI as possible. It simplifies a lot of things and avoids having to do yet-another SSO login when trying to pull up things on a device that isn't my primary machine.

That makes me lean towards the PR description option, but that has the overwrite issue you mentioned. What if we did something in-between and updated the first comment that CI posts, rather than deleting it and creating a new one? That way there is a dedicated place that won't have the overwrite issue, but we also don't trigger new notifications constantly.

I think should solve the problem 95% of the time since most devs create new PRs as drafts and wait for a successful CI run before taking the PR out of draft and pinging all of the Codeowners. After that first CI run, there shouldn't be any new notifications since CI will only be updating the existing comment. We could even post the initial CI comment as soon as CI starts to run so that the first notification arrives around the same time as the initial PR notification.

@joshdover
Copy link
Member

joshdover commented Oct 5, 2020

I was mostly focused on the build status notifications. For the other use cases, may we could expose a checkbox on the PR comment that CI can respond to updates on. Something like:

Controls

Use these checkboxes to trigger CI:

  • Merge upstream
  • Re-run the build
  • Re-run the CLA check

I believe this would work from a permissions perspective for the "jenkins test this" command on community PRs since only users with write access should be able to edit comments.

@tylersmalley
Copy link
Contributor Author

tylersmalley commented Oct 5, 2020

👍 I like the idea of re-using a single comment with checkbox controls so long as permissions aren't an issue.

We could also use Slack to trigger updates or build re-runs.

@spalger
Copy link
Contributor

spalger commented Nov 2, 2020

I've summarized what I think is the outcome of the discussion in #82335

@mfinkle
Copy link

mfinkle commented Mar 29, 2021

Do we use the Github Checks API to collect PR metrics/measure? That allows keeping data in the PR itself without using PR comments.

@tylersmalley tylersmalley added 1 and removed 1 labels Oct 11, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort labels Oct 12, 2021
@tylersmalley tylersmalley removed loe:small Small Level of Effort impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. EnableJiraSync labels Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Team:Operations Team label for Operations Team
Projects
None yet
Development

No branches or pull requests

5 participants