Skip to content
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

Add poetry and test GH action #29

Merged
merged 7 commits into from
Sep 2, 2024
Merged

Add poetry and test GH action #29

merged 7 commits into from
Sep 2, 2024

Conversation

dc-almeida
Copy link
Collaborator

Closes #14, closes #13, closes #4, closes #5

Adds poetry to project and sets up GitHub Action using poetry and pytest

@dc-almeida dc-almeida added bug Something isn't working enhancement New feature or request labels Aug 29, 2024
@dc-almeida dc-almeida self-assigned this Aug 29, 2024
Copy link
Contributor

@phackstock phackstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good to me. Two suggestions in line below.
One more thing that I would add is the __version__ attribute which most python packages implement. You can take a look at https://github.com/IAMconsortium/nomenclature/blob/18c0b1228058a30bec9898caa90df362f98ebecf/nomenclature/__init__.py#L31 how nomenclature does it.

.github/workflows/pytest.yml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Co-authored-by: Philip Hackstock <[email protected]>
Copy link
Contributor

@phackstock phackstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to be merged with one exception.
You need to explicitly set the __version__ attribute, like we do in nomenclature https://github.com/IAMconsortium/nomenclature/blob/9055d38aa2ce8d91b0d171d8c004615d6ecff58a/nomenclature/__init__.py#L31.
Once you add that you can merge this PR, no need for another review.

@dc-almeida dc-almeida merged commit c3fc90c into main Sep 2, 2024
5 checks passed
@dc-almeida dc-almeida deleted the dev branch September 2, 2024 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
2 participants