-
Notifications
You must be signed in to change notification settings - Fork 163
docs: Add inhibiting tutorial and PR workflow guidelines #1404
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
base: main
Are you sure you want to change the base?
Conversation
Thank you for contributing to the Leapp project!Please note that every PR needs to comply with the leapp-repository contribution and development guidelines and must pass all tests in order to be mergeable.
Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build.
Note that first time contributors cannot run tests automatically - they need to be started by a reviewer. It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported,
See other labels for particular jobs defined in the Please open ticket in case you experience technical problem with the CI. (RH internal only) Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra. |
MichalHe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added some clarity suggestions and typos. Do not let my review discourage you, it is a solid and valuable piece of work.
| Note that good commit message should provide information in similar the [PR | ||
| description](#pr-description). Poorly written commit messages can block the | ||
| merge of PR or proper review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Note that good commit message should provide information in similar the [PR | |
| description](#pr-description). Poorly written commit messages can block the | |
| merge of PR or proper review. | |
| Note that a good commit message should provide information similar to the [PR | |
| description](#pr-description). Poorly written commit messages can prevent the PR | |
| from being approved and merged. |
| - Fork the repository and clone your fork. | ||
| - Make your changes in a new git branch: | ||
| ```bash | ||
| git checkout -b bug/my-fix-branch main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| git checkout -b bug/my-fix-branch main | |
| git switch -c bug/my-fix-branch main |
| ```bash | ||
| git checkout -b bug/my-fix-branch main | ||
| ``` | ||
| - Commit your changes with message conforming to the our [Git commit messages guidelines](#commit-messages). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Commit your changes with message conforming to the our [Git commit messages guidelines](#commit-messages). | |
| - Commit your changes with a message conforming to the our [Git commit messages guidelines](#commit-messages). |
| ``` | ||
| - When opening a pull request, select the main branch as a base and follow our [PR description guidelines](#pr-description). | ||
| - You can open the PR as a Draft PR to prevent merging while you are still working on it. | ||
| - Wait for a review from the maintainers and address all their comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not add this sentence, sounds that we are benevolent dictators or something :D
| ## Why inhibit the upgrade process | ||
| A critical part of in-place upgrades are inhibtitors. Inhibitors are typically | ||
| used when the upgrade cannot safely continue, this can have multiple reasons: | ||
| - the upgrade is missing a required resource - for example it cannot figure |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - the upgrade is missing a required resource - for example it cannot figure | |
| - the upgrade is missing a required resource - for example, it cannot figure |
| ## What is an inhibitor | ||
| An inhibitor is a {ref}`Report<building-blocks-and-architecture:report>` with | ||
| the {py:attr}`leapp.reporting.Groups.INHIBITOR` set. There is nothing else | ||
| special about inhibitors, the inhibition is done by a dedicated actor which |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A tiny bit clearer, IMHO
| special about inhibitors, the inhibition is done by a dedicated actor which | |
| special about inhibitors, the actual inhibition (i.e., stoping the leapp parent process) is done by a dedicated actor which |
|
|
||
| ```{important} | ||
| Any Report message produced **after** the Report phase will | ||
| **not** have inhibiting effect. The details mentioned in the Report messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| **not** have inhibiting effect. The details mentioned in the Report messages | |
| **not** have the inhibiting effect. The details mentioned in the Report messages |
| ```{important} | ||
| Any Report message produced **after** the Report phase will | ||
| **not** have inhibiting effect. The details mentioned in the Report messages | ||
| will be part of the report available to the user to review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| will be part of the report available to the user to review. | |
| will be a part of the report available to the user to review. |
|
|
||
| ## Producing an inhibitor | ||
| Any actor can inhibit the upgrade process by producing a report and adding the | ||
| {py:attr}`leapp.reporting.Groups.INHIBITOR` to `leapp.reporting.Groups()`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| {py:attr}`leapp.reporting.Groups.INHIBITOR` to `leapp.reporting.Groups()`. | |
| {py:attr}`leapp.reporting.Groups.INHIBITOR` group to `leapp.reporting.Groups()`. |
| the system or upgrade configuration the use needs to deal with before moving on | ||
| with the upgrade. They are not designed to be used for general failures. To |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| the system or upgrade configuration the use needs to deal with before moving on | |
| with the upgrade. They are not designed to be used for general failures. To | |
| the system or upgrade configuration that the user needs to address before moving on | |
| with the upgrade. They are not intended to be used for general failures. To |
No description provided.