Skip to content

Commit

Permalink
words words words
Browse files Browse the repository at this point in the history
  • Loading branch information
cee-chen committed Sep 28, 2023
1 parent 507bd17 commit e3fde5d
Showing 1 changed file with 31 additions and 3 deletions.
34 changes: 31 additions & 3 deletions wiki/eui-team-processes/upgrading-kibana.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
3. [Resolving errors](#resolving-errors)
* [Snafus requiring backports](#snafu)
4. [Merging the PR](#merging-the-pr)
* [FAQ for Kibana teams](#faq-for-kibana-teams)

## Getting started

Expand Down Expand Up @@ -179,7 +180,7 @@ Once CI is passing green, mark your draft PR as "Ready for review".
### Leaving comments

* Mention/ping any teams that are waiting on features to be available.
* Upgrades that result in significant changes to tests and/or snapshots will often require codeowner approval from several teams. When possible, head-off questions and small change requests by leaving review comments explaining changes and adhering code styles typical of the plugin.
* Upgrades that result in significant changes to tests and/or snapshots will often require codeowner approval from several teams. When possible, head-off questions and small change requests by leaving review comments explaining changes and adhering to code styles typical of the plugin.

### Keeping your PR up to date

Expand All @@ -188,6 +189,33 @@ Once CI is passing green, mark your draft PR as "Ready for review".

### Waiting for approvals/admin merges

* Team sizes and priorities vary, so allow each team 1-2 days before escalating the review notification. After the waiting period, ping all teams with outstanding reviews containing non-snapshot code changes on Slack, letting them know that the review is for an EUI upgrade PR.
* When the PR opens, ping all CODEOWNER teams immediately on Slack to give teams as much heads up as possible, as team sizes and priorities vary. We should generally let teams know an approximate amount of time they'll have before we request an admin merge (e.g. EOD Friday).
* Once all approvals are in and CI is still passing green, smash that merge button :boom:
* If, after several days and nudges, review(s) remain outstanding, we should ask KibanaOps to admin merge the PR without all approvals (provided that the review(s) are for minor changes we feel confident in, e.g. snapshot updates with an expected diff).
* If, by the end of the week, review(s) remain outstanding, we should ask KibanaOps to admin merge the PR without all approvals (provided CI is passing and that that the review(s) are for minor changes we feel confident in, e.g. snapshot updates with an expected diff).
### FAQ for Kibana teams
#### Q: I've been pinged as a CODEOWNER for a EUI upgrade PR. What should I do?

If you've been pinged, this means that some code belonging to your team has been changed as a result of upstream EUI changes. This can be due to several reasons:
- A snapshot change due to underlying DOM or CSS/class changes in EUI
- A breaking change that required your code to be updated in order to continue functioning as before
- A (typically) minor cleanup change that EUI noticed in your code
We ask for a code review of the files owned by your team (not the entire PR) to ensure that it looks reasonable. QA is optional - in general, we rely on tests and feature freeze QA to catch any UI issues that may have slipped past (hopefully few to none).
We do encourage manual QA of your team's app/plugin if the changes are large enough, or if the impacted component/UI is important enough to warrant concern about any regressions or broken behavior. In general, EUI tries to call out any larger or riskier changes at the top of our PR description to help narrow down QA/smoke testing.

#### Q: I've caught an issue that we think is caused by the EUI upgrade - what do I do?

If the issue only occurs on the upgrade PR branch and not in Kibana main, please feel free to alert us in a GitHub comment and we'll look into resolving it ASAP.
If you've caught an issue post-merge that you think is the a result of an EUI upgrade, we're always more than happy to investigate and find a resolution - please feel free to reach out to us in the EUI Slack channel.
EUI tries (when humanly possible) not to merge in larger or riskier PRs close to feature freeze dates, to avoid issues shipping into production releases.
#### Q: Why are requested review turnaround times so short?
EUI upgrade PRs have shorter (~weekly) turnarounds, which is an challenging necessity for our team. We own upgrades that touch many teams' files (usually snapshots), and holding up PRs can at times block certain teams from needed features, or end up causing a pile-up of massive changelogs later.

If you can't review in time, that's fine, just let us know. We're always happy to investigate issues post-merge if necessary.

0 comments on commit e3fde5d

Please sign in to comment.