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

Changes build system to flit #7

Merged
merged 13 commits into from
Nov 18, 2022
Merged

Conversation

AlejandroFernandezLuces
Copy link
Collaborator

Overview

To standarize the project, I moved the build system to flit, which uses the pyproject.toml standard. This file allows the project building to be tool independent.

@RobPasMue
Copy link
Contributor

RobPasMue commented Nov 18, 2022

cache-dependency-path: requirements_test.txt has to be adapted to use pyproject.toml in the workflow ;)

@codecov-commenter
Copy link

codecov-commenter commented Nov 18, 2022

Codecov Report

Merging #7 (b245bc1) into main (eafb8fb) will increase coverage by 0.22%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main       #7      +/-   ##
==========================================
+ Coverage   83.56%   83.78%   +0.22%     
==========================================
  Files           1        2       +1     
  Lines          73       74       +1     
==========================================
+ Hits           61       62       +1     
  Misses         12       12              
Impacted Files Coverage Δ
pytest_pyvista/pytest_pyvista.py 83.56% <ø> (ø)
pytest_pyvista/__init__.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@AlejandroFernandezLuces AlejandroFernandezLuces marked this pull request as ready for review November 18, 2022 10:25
@AlejandroFernandezLuces
Copy link
Collaborator Author

I'm not sure how to handle the cache option in actions/setup-python@v4, since now we are managing all of the project dependencies with flit. Currently, I removed it because I don't know if there is any way to keep a cache of the packages with flit.

Is there any option to cache the installed packages using flit @RobPasMue ?

Copy link
Contributor

@RobPasMue RobPasMue left a comment

Choose a reason for hiding this comment

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

I think that's the only thing you need @AlejandroFernandezLuces :)

.github/workflows/ci_cd.yml Outdated Show resolved Hide resolved
Co-authored-by: Roberto Pastor Muela <[email protected]>
@RobPasMue
Copy link
Contributor

The way cache-dependency-path works is by checking the checksum of whatever file you provide. If it doesn't change, it will use the stored cache with that hash/checksum. Otherwise, it'll generate a new one. Just keep that in mind.

I also saw that you were installing it using flit directly. I recommend that you keep using pip for doing the install process. That way you can make use of the cache: 'pip' option. I left you another suggestion.

@MaxJPRey
Copy link
Contributor

Thanks @RobPasMue for looking at the PR. Much appreciated.

Co-authored-by: Roberto Pastor Muela <[email protected]>
Co-authored-by: Roberto Pastor Muela <[email protected]>
]

[project.entry-points."pytest11"]
# Location of the plugin module, in this case ./pytest_pyvista/pytest_pyvista.py
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment might be useful for future maintenance. Thanks for adding it.

Copy link
Contributor

@MaxJPRey MaxJPRey left a comment

Choose a reason for hiding this comment

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

LGTM.

@AlejandroFernandezLuces AlejandroFernandezLuces merged commit e26694d into main Nov 18, 2022
@AlejandroFernandezLuces AlejandroFernandezLuces deleted the maint/flit-build branch November 18, 2022 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants