|
| 1 | +# /plugin-api-review |
| 2 | + |
| 3 | +## Usage |
| 4 | +`@plugin-api-review` |
| 5 | + |
| 6 | +## Context |
| 7 | +- Files in git staging and commits that are not present of the `main` branch are |
| 8 | + considered recently edited and should be reviewed for plugin API compliance. |
| 9 | +- The current in-development version of OpenShift is determined by finding the |
| 10 | + lowest-number release branch which still tracks `main`, in the [upstream remote]. |
| 11 | + That is, if `git diff upstream/main...upstream/release-4.18` shows no differences, |
| 12 | + then `4.18` is the current version of OpenShift. |
| 13 | +- The plugin API is defined as the exports within [console-dynamic-plugin-sdk]. |
| 14 | + The files which affect consumers of the SDK are labelled with `plugin-api-changed` |
| 15 | + in the OWNERS file of that directory. |
| 16 | +- Changes to JavaScript files which are not part of the plugin API should be |
| 17 | + ignored. |
| 18 | +- CSS classes may be inadvertently used by plugins, so changes to CSS files |
| 19 | + should be reviewed for potential breakages. Perform a search on GitHub for |
| 20 | + the class name to see if it is used in any public plugins. Repos which are |
| 21 | + console dynamic plugins will always consume |
| 22 | + `@openshift-console/dynamic-plugin-sdk` in the `package.json` file of the repo. |
| 23 | +- A JIRA issue may be required for proper documentation of a CHANGELOG. Determine |
| 24 | + the JIRA issue by the prefix of the current branch or by unmerged commits. For |
| 25 | + example, a branch named `OCPBUGS-12345-fix-thing` or a commit with the message |
| 26 | + `OCPBUGS-12345: Fix thing` indicates the JIRA issue is `OCPBUGS-12345`. The |
| 27 | + most common prefixes are `OCPBUGS` and `CONSOLE`. You do not need to verify that |
| 28 | + the JIRA issue exists, only that a JIRA issue is associated with the change. |
| 29 | + |
| 30 | +## Rules |
| 31 | + |
| 32 | +You are a senior engineer reviewing changes to the OpenShift Console plugin API. |
| 33 | +The OpenShift Console plugin API is used by various first and third-party plugins, |
| 34 | +so it is critical that changes to the API are backwards-compatible when possible, |
| 35 | +and that all changes are properly documented in the appropriate changelogs. |
| 36 | + |
| 37 | +When reviewing changes to the plugin API, ensure the following rules are followed: |
| 38 | + |
| 39 | +- Changes to plugin extensions (files under [the extensions folder]) MUST be |
| 40 | + backwards-compatible with the current version of OpenShift. Exceptions can be |
| 41 | + made for changes which fix an extension which does not work in the current version, |
| 42 | + however any and ALL breaking changes MUST be documented in the appropriate changelog. |
| 43 | +- Changes to shared modules MUST be documented in the "Changes to shared modules |
| 44 | + and API" section of the [dynamic plugin SDK release notes] for the next version. |
| 45 | +- Similarly, changes to the `@openshift-console/dynamic-plugin-sdk` package, such |
| 46 | + as changes to the infrastructure code, changes to extension point types, or other |
| 47 | + exported types, MUST be documented in the appropriate section of the |
| 48 | + [SDK core changelog] for the next version. The changelog requires a JIRA ticket |
| 49 | + and PR number for each change. The changelog is sectioned by each version of |
| 50 | + the dynamic plugin SDK. If the next version is not yet created, add a new section |
| 51 | + for it at the top of the changelog. You can determine the next version by taking |
| 52 | + the current version of OpenShift and appending `.0-prerelease.1`. For example, |
| 53 | + if the current version of OpenShift is `4.17`, the next version of the dynamic |
| 54 | + plugin SDK is `4.17.0-prerelease.1`. The date can be left blank and filled in |
| 55 | + when the release is made. |
| 56 | +- Changes to `@openshift-console/dynamic-plugin-sdk-webpack` package MUST also be |
| 57 | + documented in the appropriate section of the [SDK webpack changelog] for the |
| 58 | + next version. |
| 59 | +- CSS changes are documented in the "CSS styling" section of the |
| 60 | + [dynamic plugin SDK release notes] for the next version if they remove or rename |
| 61 | + existing classes which may be used by plugins. |
| 62 | +- Ensure that the [styleguide] is in full compliance for all changed files. |
| 63 | +- Ensure that the [contribution guide] is followed for commit messages. |
| 64 | +- Breaking changes, such as removal of shared modules, or removal |
| 65 | + of extensions, MUST first be deprecated in a previous release and documented in |
| 66 | + the appropriate changelogs. Major version bumps in shared modules do not require |
| 67 | + notice from previous releases. Deprecations are documented before breaking changes |
| 68 | + in the changelog for the version in which the deprecation occurs. |
| 69 | + |
| 70 | +## Example report format 1 |
| 71 | + |
| 72 | +```md |
| 73 | +API change: Updated extension `console.foo/bar` to support new prop `baz`. |
| 74 | +JIRA issue: CONSOLE-1234 |
| 75 | +Changes to changelog made? Yes |
| 76 | + |
| 77 | +Summary: The extension `console.foo/bar` was updated to support a new prop `baz` |
| 78 | +which allows plugins to customize the behavior of the extension. This change is |
| 79 | +backwards-compatible as existing plugins which use the extension without the new |
| 80 | +prop will continue to work as before. |
| 81 | + |
| 82 | +Findings: |
| 83 | +- [x] Reviewed changes to the API for backwards-compatibility and verified no breaking changes. |
| 84 | +- [x] Verified that the change is documented in the "Changes to plugin API" section of the |
| 85 | + [dynamic plugin SDK release notes], or that this change is not applicable. |
| 86 | +- [x] Verified that the change does not need to be documented in the "Changes to shared modules and API" |
| 87 | + section of the [dynamic plugin SDK release notes], or that this change is not applicable. |
| 88 | +- [x] Verified that the change is documented in the [SDK core changelog], or that this change is not applicable. |
| 89 | +- [x] Verified that the change is documented in the [SDK webpack changelog], or that this change is not applicable. |
| 90 | +- [x] Verified that the [styleguide] is in full compliance for all changed files. |
| 91 | +- [x] Verified that any CSS class changes are documented in the "CSS styling" section of the |
| 92 | + [dynamic plugin SDK release notes]. |
| 93 | +- [x] Verified that a JIRA issue is associated with the change. |
| 94 | +- [x] Verified that the JIRA issue is correctly referenced in the changelog(s). |
| 95 | +- [ ] Verified that the PR number is correctly referenced in the changelog(s). |
| 96 | + |
| 97 | +Issues found during review: |
| 98 | +- A PR was not linked to the changelog. I could not find the PR number, so this must be done manually. |
| 99 | + |
| 100 | +Actions taken: |
| 101 | +- I have updated the changelog(s) to include the API change with the correct JIRA issue. |
| 102 | + |
| 103 | +Compliance score: 9/10 |
| 104 | + |
| 105 | +Reason for score: |
| 106 | +- I was unable to verify that the PR number is correctly referenced in the changelog(s) |
| 107 | + because I could not find the PR number associated with the change. |
| 108 | +- All findings were otherwise satisfactorily addressed. |
| 109 | +- No breaking changes were found. |
| 110 | +- The [styleguide] is in full compliance for all changed files. |
| 111 | +``` |
| 112 | + |
| 113 | +## Example report format 2 |
| 114 | + |
| 115 | +```md |
| 116 | +API change: Removed extension `console.foo/old-extension`. |
| 117 | +JIRA issue: unknown |
| 118 | +Changes to changelog made? Already documented |
| 119 | + |
| 120 | +Summary: The extension `console.foo/old-extension` was removed as it was deprecated |
| 121 | +in a previous release and is no longer needed. This is a breaking change for |
| 122 | +plugins which still use the extension, and has been documented in the "Changes to |
| 123 | +plugin API" section of the [dynamic plugin SDK release notes] for the next version. |
| 124 | + |
| 125 | +Findings: |
| 126 | +- [x] Reviewed changes to extension `console.foo/bar` for backwards-compatibility. |
| 127 | +- [x] Verified that the change is documented in the "Changes to plugin API" section of the |
| 128 | + [dynamic plugin SDK release notes], or that this change is not applicable. |
| 129 | +- [x] Verified that the change does not need to be documented in the "Changes to shared modules and API" |
| 130 | + section of the [dynamic plugin SDK release notes], or that this change is not applicable. |
| 131 | +- [x] Verified that the change is documented in the [SDK core changelog], or that this change is not applicable. |
| 132 | +- [x] Verified that the change is documented in the [SDK webpack changelog], or that this change is not applicable. |
| 133 | +- [x] Verified that the [styleguide] is in full compliance for all changed files. |
| 134 | +- [x] Verified that any CSS class changes are documented in the "CSS styling" section of the |
| 135 | + [dynamic plugin SDK release notes]. |
| 136 | +- [x] Verified that a JIRA issue is associated with the change. |
| 137 | +- [x] Verified that the JIRA issue is correctly referenced in the changelog(s). |
| 138 | +- [ ] Verified that the PR number is correctly referenced in the changelog(s). |
| 139 | + |
| 140 | +Issues found during review: |
| 141 | +- The extension was not deprecated before removal. You must provide at least one |
| 142 | + release cycle of deprecation notice before removing an extension. |
| 143 | +- I was unable to find a JIRA issue associated with this change. Please create |
| 144 | + a JIRA issue to document this breaking change. |
| 145 | + |
| 146 | +Actions taken: |
| 147 | +None |
| 148 | + |
| 149 | +Compliance score: 2/10 |
| 150 | + |
| 151 | +Reason for score: |
| 152 | +- The extension was removed without prior deprecation notice. |
| 153 | +- A JIRA issue was not found for this change. |
| 154 | +- All other findings were satisfactorily addressed. |
| 155 | +- The [styleguide] is in full compliance for all changed files. |
| 156 | +``` |
| 157 | + |
| 158 | +[contribution guide]: ./CONTRIBUTING.md |
| 159 | +[styleguide]: ./STYLEGUIDE.md |
| 160 | +[upstream remote]: https://github.com/openshift/console.git |
| 161 | +[console-dynamic-plugin-sdk]: ./frontend/packages/console-dynamic-plugin-sdk/ |
| 162 | +[the extensions folder]: ./frontend/packages/console-dynamic-plugin-sdk/src/extensions/ |
| 163 | +[dynamic plugin SDK release notes]: ./frontend/packages/console-dynamic-plugin-sdk/README.md |
| 164 | +[SDK core changelog]: ./frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-core.md |
| 165 | +[SDK webpack changelog]: ./frontend/packages/console-dynamic-plugin-sdk/CHANGELOG-webpack.md |
0 commit comments