Thanks for looking into making dbt-sugar
better! We have some loosely defined rules and preferences to make the contribution a bit smoother. Don't let those deter you from contributing though, most of them can absolutely be fixed in the PR process, and if anything seems obscure feel free to reach out on the discord channel Discord.
- It usually starts with creating an issue (reporting a bug, or discussing a feature). In fact, even if you don't know how to code it or don't have the time, you're already helping out by pointing out potential issues or functionality that you would like to see implemented.
- Creating an issue is not necessary!
- See an already published issue that you think you can tackle? Drop a line on it and get cracking or ask questions on how you can help, it's generally a good way to make sure you'll hit home right away. It's not fun to do a lot of work and then find out the proposed change doesn't fit with the maintainer's goals.
It would be really nice if you could follow a few of the guidelines around branch and PR naming, but don't let that deter you from contributing (as we said earlier these will be fixable during the PR process --but it may be annoying to you to get comments that could sound like nitpicking).
Below is the preferred format:
feat/<issue_number_if_applicable>/my_awesome_feature
fix/<issue_number_if_applicable>/describe_what_you_fix
refactor/<issue_number_if_applicable>/describe_what_you_refactor
docs/<issue_number_if_applicable>/what_is_changing_what_you_are_explaining
We follow conventional-commits to standardise commits, and help with CHANGELOG.md generation. If you feel like you don't want to bother with it it's ok, we squash all commits and we ensure the PR is named with a conventional commit pattern when we merge your branch.
PR linting will fail if the PR isn't name with one of prefixes below. Don't worry though, we'll get that sorted before merging 🎉
Here is what your PR names should ideally look like:
feat: my awesome feature
fix: relax version requiremtents on pandas
docs: document new feature
refactor: move cleanups into its own class
Use the template provided for you as a guide to cover most aspects of your PR. Feel free to delete stuff you think isn't relevant. The template is just a guide.
Please follow generally accepted git best practices by using:
- descriptive commit messages,
- imperative mood
TIP: ✨ it generally helps to say in your head "This commit will..." and then start writing. You'll end up with the right phrasing and it will trigger you to think about a useful description.
This leads to you writing commit messages such as implement my awesome feature
instead of implementing feature a
or implemented feature a
which are less conventional.
We'll squash your PR at merge time.
In this project we use towncrier
to generate our changelog. When you make a PR, no-one better than you should be able to describe your code change! Towncrier works with news fragments. For each PR that proposes changes that need to be talked about in the changelog you should create a news fragment file in the following way:
cd changelog/
- using your editor of choice create a file following the format below. Make sure to pick the category (feature, fix or misc) wisely:
<issue_or_pr_number>.<feature/fix/misc>.md
- save it and you're done. The release manager will take care of aggregating the changelog at release time
Fork the repo and get going. If you're not too experienced with forks, feel free to shoot us a DM on Discord and join the #dbt-sugar-dev
channek. Here's a quick guide though:
Shit how do we do this fork thing?
For official guidelines check the GitHub documentation
-
Create a fork from this repo in your accout by clicking the "Fork" button
-
Clone the fork on your machine via ssh or http depending on how you like to authenticate and all that. For example:
git clone [email protected]:<your_username>/dbt-sugar.git
-
Modify your code locally.
As you would for a regular cloned repo,
git branch
,git add
,git commit
,git push
. The push will go to your fork remote which is totally fine. -
Create a Pull Request from your own forked repository.
When the time comes to merge your code back on the original repo. Go to your fork's GitHub page, click the "Pull Request" button and choose to merge into the original repo.
-
Keep your fork up to date. This is required if you want to integrate new features from master into your branch or if you want to resolve upstream conflicts. Don't worry too much about it the maintainers will help you with this if you don't really feel 100%.
Say, you want to update or pull the original repo you can add the original repo as another remote called
upstream
(actually you can name if whatever you want but usually that's how people name this) to your config like so:git remote add upstream https://github.com/bitpicky/dbt-sugar.git
We have configured pre-commit
in our repository to ensure that all things related to linting, code formatting, and import sorting is taken care of ahead of the PR. We want to make sure the review process can focus on the code change that matter rather than petty discussion about line-length, indentation etc.
It is recommended to install pre-commit
on your machine if you have not done so already by doing the following:
pip install pre-commit
If you don't want to bother, that's also OK because we also have pre-commit.ci on the PR side of things, you might just get annoying test failures that you could have taken care of earlier. But no worries, we'll help you out!
-
We format our code with the
black
formatter. The pre-commit hooks will attempt to fix your formatting issues for you but it's "annoying" so you can set upblack
to format your code on save in your IDE and then you'll never have a problem again. The good thing with black is that we don't ever have to discuss formatting and style while reviewing your PR which is soooooo much nicer! 😍 -
flake8
is also part of our pre-commit config. If you do not have it installed as a linter in your IDE or you ignore its recommendationpre-commit
will fail. flake8 in precommits does not automatically fix your files so it's better to enforce it's advice during developments but that's up to you. -
Finally, trailing whitespace and empty new line at end of file will also be enforced for you by
pre-commit
either during PR time or locally if you have set it up according to our guidelines.
- It is recommended to use Type Hinting and have
mypy
enabled as your linter. Most IDE's have an extension or a way to help with this. Typing isn't necessary but really, really preferred. The mainteners might therefore make suggestions on how to implement typing or will enfore it for you directly in your branch. - Mypy is also part of our
pre-commit
and it should alert you if you have any issues with type hints.
The whole package is managed using Poetry. It's really really good and ensures reproducibility. **If this is holding you up from contributing feel free to shoot us a DM on Discord and we can see what are other ways. It's likely that pip
would understand the pyproject.toml
file and then you'll be free to set up a virtual environment of your choice and get going.
- Install Poetry on your system --I personally recommend installing it via
pipx
but that's entirely up to you. cd
to your fork and runpoetry install
in the root folder of the repo. Poetry will create a virtual environment for python, install dependencies and installdbt-sugar
in editable mode so that you can directly run and testdbt-sugar
as you develop without having to install over and over.- The easiest way to activate the poetry virtual env is to call
poetry shell
it'll wrap around your shell and and activate the dbt-sugar venv, to deactivate or get out of the shell simply typeexit
. More info on the Poetry documentation website
If you're comfortable writing tests for your features, we use the pytest
framework. It'll be installed automatically when you set up your poetry environment. If you don't know how to write tests that's fine, we'll work it out with you during the review process 💪🏻
Have a look into the tests/
folder for how the tests are written and if you want to trigger tests locally you can do so from the root of the repo with
pytests tests/
Thanks for your interest in contributing. We really appreciate it and hope those guidelines don't sound too much. If you have any questions feel free to shoot us a message on our discord server Discord.