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

Migrate to pixi #12

Merged
merged 38 commits into from
Jun 25, 2024
Merged

Migrate to pixi #12

merged 38 commits into from
Jun 25, 2024

Conversation

kklein
Copy link
Collaborator

@kklein kklein commented Jun 18, 2024

I think that we should add a 'development' section in the docs as a follow-up to this PR; explaining how one can locally

  • install the environment
  • install and run pre-commit hooks and
  • run the tests

@kklein kklein changed the title Draft usage of pixi in ci.yml. Migrate to pixi Jun 24, 2024
Copy link

codecov bot commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.67%. Comparing base (390d53c) to head (5ee2860).
Report is 56 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #12   +/-   ##
=======================================
  Coverage   94.67%   94.67%           
=======================================
  Files          14       14           
  Lines        1295     1295           
=======================================
  Hits         1226     1226           
  Misses         69       69           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kklein kklein marked this pull request as ready for review June 24, 2024 12:51
@kklein
Copy link
Collaborator Author

kklein commented Jun 24, 2024

FYI @DelongChenQC, @MatthiasLoefflerQC (couldn't find Arsenyi's GitHub handle)

uses: quantco/pre-commit-conda@v1
environments: default lint
- name: pre-commit
run: pixi run pre-commit-run --color=always --show-diff-on-failure

mypy-type-checks:
Copy link
Member

Choose a reason for hiding this comment

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

When properly checking it in pre-commit (see other comment), we don't need this here as it's already being checked by the pre-commit hook.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed it!

.pre-commit-config.yaml Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
pixi.toml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
pixi.toml Outdated Show resolved Hide resolved
pixi.toml Outdated Show resolved Hide resolved
Co-authored-by: Pavel Zwerschke <[email protected]>
Co-authored-by: Pavel Zwerschke <[email protected]>
Copy link
Contributor

@FrancescMartiEscofetQC FrancescMartiEscofetQC left a comment

Choose a reason for hiding this comment

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

Looking good, I think it's important to add in the README how one can build the environment and the basics for development but maybe we can leave it for another PR.

pixi.toml Outdated Show resolved Hide resolved
@kklein
Copy link
Collaborator Author

kklein commented Jun 24, 2024

The Windows tests are failing - I created an issue to follow up on it after this PR:
#24

pavelzw
pavelzw previously approved these changes Jun 24, 2024
Copy link
Contributor

@FrancescMartiEscofetQC FrancescMartiEscofetQC left a comment

Choose a reason for hiding this comment

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

LGTM!

.github/workflows/ci.yml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
Co-authored-by: Pavel Zwerschke <[email protected]>
@kklein kklein merged commit ed92851 into main Jun 25, 2024
11 checks passed
@kklein kklein deleted the pixi branch June 25, 2024 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants