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.6/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
We have some required dependencies you should have installed on your system (either your local computer or a docker container) to get ready to write some code:
Required dependencies:
- virtualenv
- python
- pre-commit
- git
- podman or docker
Optional dependencies:
The commands below will create a python3 virtual environment with all the
necessary dependencies installed, images built, .env file created, 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
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.
The documentation for our python code should follow the 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.
For building the rpms locally, you will need:
- podman or a docker
make rpmsFor building the copr locally, you will need:
- podman or a docker 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.