The following is a set of guidelines for contributing to Convert2RHEL codebase, which are hosted in the OAMG Organization on GitHub. These are mostly guidelines, not rules.
Below are a list of things to keep in mind when developing and submitting contributions to this repository.
- All python code must be compatible with versions 2.7/3.6.
- The code should follow linting from pylint.
- All commits should have passed the pre-commit checks.
- Don't change code that is not related to your issue/ticket, open a new issue/ticket if that's the case.
If you are not sure on how GitHub works, you can read the quickstart guide from GitHub to introduce you on how to get started at the platform. GitHub Quickstart - Hello World.
If you never used git before, GitHub has a nice quickstart on how to set it
up and get things ready. GitHub Quickstart - Set up
Git
Forking is necessary if you want to contribute with Convert2RHEL, but if you are unsure on how this work (Or what a fork is), head out to this quickstart guide from GitHub. GitHub Quickstart - Fork a repo
As an additional material, check out this Red Hat blog post about What is an open source upstream?
Check out this guide from GitHub on how to collaborate with pull requests. This is an in-depth guide on everything you need to know about PRs, forks, contributions and much more. GitHub - Collaborating with pull requests
Note
This has only been tested with Podman
For an easy setup where you can get up and running within a minute, we have a dedicated development container setup with VS Code. This creates a container with extensions and allows you to run convert2rhel and tests without drawbacks.
This does not work with VSCodium as Dev Container is deliberately limited to only work with Microsoft VS Code binaries.
Note: we need to run tests in a container as convert2rhel is volatile, a small mistake in tests can make modifications on your system.
To get started with a Dev Container, you need to install the Dev Container extensions in VS Code. You can follow the link or install this ID
ms-vscode-remote.remote-containers
Once the extension is installed, all you need to do is open up the repository inside a container, pressing F1 or
Ctrl-Shift-P and starting the following will open up the container for you
Dev Containers: Open Folder in Container
You are now inside a container as indicated by the bottom left box. There is an extension for tests in the left
navigation that auto-discovers tests that you can run. This will be much faster than developing using make tests
We have some required dependencies you should have installed on your system (either your local computer or a container) to get ready to write some code:
Required dependencies:
- virtualenv
- python
- pre-commit
- git
- podman
Optional dependencies:
The commands below will create a python3 virtual environment with all the
necessary dependencies installed, images built, and setup pre-commit hooks.
Beware this command can take a while to finish, depending on your internet connection.
make installYou can run the linters against the codebase to get a complete summary of what is not standardized in the current codebase (as a whole)
make lint # Runs inside a centos8 containeror locally, with:
make lint-locallyPre-commit is an important tool for our development workflow, with this tool we
can run a series of pre-defined hooks against our codebase to keep it clean and
maintainable. Here is an example of output from pre-commit being run:
(.venv3) [r0x0d@fedora convert2rhel]$ pre-commit run --all-files
Format code (black)......................................................Passed
isort....................................................................Passed
Fix End of Files.........................................................Passed
Trim Trailing Whitespace.................................................Passed
Check JSON...........................................(no files to check)Skipped
Check Toml...............................................................Passed
Check Yaml...............................................................Passed
Check for merge conflicts................................................Passed
We automatically run pre-commit as part of our CI infrastructure as well. If you have a PR it will run and see if everything passes. Sometimes there may be an outage or unexpected result from pre-commit, if that happens you can create a new comment on the PR saying:
pre-commit.ci run
Install pre-commit hooks to automatically run when doing git commit.
# installs pre-commit hooks into the repo (included into make install)
pre-commit install --install-hooksRunning pre-commit against our files
# run pre-commit hooks for staged files
pre-commit run
# run pre-commit hooks for all files in repo
pre-commit run --all-files
# bypass pre-commit hooks
git commit --no-verifyAnd lastly but not least, if you wish to update our hooks, we can do so by running the command:
# bump versions of the pre-commit hooks automatically to the latest available
pre-commit autoupdateIf you wish to learn more about all the things that pre-commit can do, refer
to their documentation on how to use
pre-commit.
Tests are an important part of the development process, they guarantee to us that our code is working in the correct way as expected, and for Convert2RHEL, we separate these tests in two categories.
- Unit testing
- Integration testing
If you're in doubt on how to make a test, you can read this great article from Real Python - Getting Started With Testing in Python to get a better idea between the differences between these tests and how they are usually made.
Also, you can check out our unit_tests/README.md file to get a better understanding on how to write, run and see the coverage of the unit tests.
To learm more about our integration tests, see tests/integration/README.md.
- Use the present tense ("Add feature" not "Added feature")
- Use the imperative mood ("Move cursor to..." not "Moves cursor to...")
- Limit the first line to 72 characters or fewer
- If you are fixing a GitHub issue, include something like "Closes #xyz"
To know more about best practices for commit messages, go ahead and read How to Write a Git Commit Message.
For Convert2RHEL we use the PEP 8 -- Style Guide for Python Code as well some linters/formatters that are run directly from pre-commit.
Don't worry, most of the code formatting and styleguide is handled by black, an awesome formatter for python code.
Code should be self-documenting, but it does not take into consideration how the functions should be used, what the scope of the classes are, or what the variable is to be used for. We follow PEP 8 -- Documentation Strings or more precisely the PEP 257 -- Docstring Conventions and preferably all functions should contain at least a minimum docstring to identify what that function is used for.
When writing the docstrings, imagine onboarding a new person to the codebase half a year down the line and what they need to know to understand the function and any edge-cases.
We make use of type comments where possible to get type hinting, since we cannot use type hints due to having to support python 2.7. You can find the type comments style in PEP-0484#type-comments
Additional technical information is located on the wiki. When you add a new feature that others might need to plug into or affects how their code should be written in the future, please add a new page there and link it to the toplevel Topics section.
For building the rpms locally, you will need:
- podman
make rpmsFor building the copr locally, you will need:
- podman installed
- have an account at https://copr.fedorainfracloud.org/api/, copy copr config and paste it to the repo root as .copr.conf
- request build permissions at https://copr.fedorainfracloud.org/coprs/g/oamg/convert2rhel/permissions/
WARNING: Do not bump the Release in packaging/convert2rhel.spec. Otherwise, this version will be automatically installed during yum install
make copr-buildWe found a bug that happened during the handling of a SIGINT
signal (or pressing CTRL + C). The bug occurred when a user pressed that
combination of keys during any step of the conversion before the point of no
return. The Ctrl-C started the rollback process and failed in the first
yum API call to check whether or not some packages were installed on the system.
The problem is that when we enter rollback after a SIGINT, librpm handles the
signal in its code (without any possibility to change this). The librpm handler
detects that a SIGINT was raised, decides that the execution of the process
should stop, and sends 1 as the program's exit code. One way to bypass this
problem is not to call yum methods that actually trigger librpm in the
process, instead, calling the yum binary or any other binary that achieves
the same goal. By calling yum via call_yum_cmd() or run_subprocess(),
a separate process is started and the SIGINT is handled by that separate
process rather than the main convert2rhel process.
For example, checkout
PR#411
which introduced the fix for this bug, as well, some explanation on why the code was
changed from calling the yum API to calling the rpm command.
Here's the ticket that originated this issue: OAMG-5756.
Lastly, a minimal reproducer from @abadger where you can see the issue.
#!/usr/bin/python2 -tt
import time
from yum import YumBase
from rpmUtils.miscutils import checkSignals
yb = YumBase()
try:
#yb.doConfigSetup(init_plugins=False)
yb.runTransaction(False)
except (BaseException, SystemExit, Exception) as e:
print('We were able to catch an exception from runTransaction!')
print(type(e))
print(e)
print('Sleeping')
time.sleep(3)
# Hit Ctrl-C
# If KeyboardInterrupt is raised, chances are that this did not cause the
# problem
try:
checkSignals()
except (BaseException, SystemExit, Exception) as e:
print('We were able to catch an exception from checkSignals!')
print(type(e))
print(e)
finally:
print('In the finally block')
# If the previous try: except exits immediately without message, then the
# issue occurred.
# If it raises a KeyboardInterrupt traceback then we're okay.
# If it prints out anything then we're either okay or encountering a different
# behaviour.