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

Minor refactoring #243

Merged
merged 3 commits into from
Sep 24, 2022
Merged

Minor refactoring #243

merged 3 commits into from
Sep 24, 2022

Conversation

maresb
Copy link
Contributor

@maresb maresb commented Sep 22, 2022

Here are a few really minor refactoring things I did in preparation for vendoring Poetry in #240, which should be very straightforward to review.

  1. Fix typo in docs: pip was listed twice as a dependency in CONTRIBUTING.md.
  2. The variable dep was being used as both Dependency and LockedDependency types, so to make type checkers happier, I renamed the LockedDependency instances of dep to locked_dep.
  3. When initializing the HashModel object, it is possible (from the typing perspective if not in practice) that link.hash_name and link.hash are None. Thus HashModel(**{link.hash_name: link.hash}) would expand to HashModel(None=None), which would result in TypeError: keywords must be strings. I rewrote this bit of code to be a bit lengthier, but more explicit and robust.

@netlify
Copy link

netlify bot commented Sep 22, 2022

Deploy Preview for conda-lock ready!

Name Link
🔨 Latest commit d280dcd
🔍 Latest deploy log https://app.netlify.com/sites/conda-lock/deploys/632cc995631f170009b27534
😎 Deploy Preview https://deploy-preview-243--conda-lock.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@maresb maresb marked this pull request as ready for review September 23, 2022 22:55
@mariusvniekerk mariusvniekerk merged commit d709e9e into conda:main Sep 24, 2022
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