Skip to content

Conversation

@emmettbutler
Copy link

  • Adds support for the hatchling and flit_core PEP517 build backends
  • Fixes an infinite recursion bug by avoiding package name collisions in overlay
  • Adds a --py argument to the CLI allowing the user to specify the Python version to be used in the built shell
  • Pins nixpkgs to improve reproducibility

@cript0nauta
Copy link
Owner

Pins nixpkgs to improve reproducibility

I think this change shouldn't be necessary. If you use the --nixpkgs option, the expression will be instantiated with <nixpkgs> pinned to the desired version: https://github.com/cript0nauta/pynixify/blob/main/pynixify/nixpkgs_sources.py#L135

@cript0nauta
Copy link
Owner

Adds a --py argument to the CLI allowing the user to specify the Python version to be used in the built shell

This change looks good. Would you be able to create a new PR that only includes this fix, without any changes to formatting or import order?

Also, please make sure to use the escape_string function when including a string inside a Nix expression.

@cript0nauta
Copy link
Owner

Fixes an infinite recursion bug by avoiding package name collisions in overlay

Do you have an example of when this bug is triggered? Maybe we should create a test in the command test suite that reproduces it.

@cript0nauta
Copy link
Owner

Adds support for the hatchling and flit_core PEP517 build backends

This changeset looks interesting, though I'm not familiar with PEP517. I'll need some time to review the changes since they're larger than the other fixes.

Thanks again for your contributions!

@emmettbutler
Copy link
Author

emmettbutler commented May 20, 2023

@cript0nauta thanks for the quick feedback! i broke out the pep517 changes into this pull request for your review #65

here's a pull request implementing the new cli option #66

here's a pull request showing how to replicate the recursion bug i encountered emmettbutler#4

thanks again! i'll look forward to your feedback.

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.

2 participants