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

A couple of small fixes that I noticed #2

Merged
merged 3 commits into from
Jul 7, 2022
Merged

A couple of small fixes that I noticed #2

merged 3 commits into from
Jul 7, 2022

Conversation

seanh
Copy link
Contributor

@seanh seanh commented Jul 6, 2022

No description provided.

Comment on lines -26 to -31
@property
@lru_cache
def abspath_hash(self):
hash_ = hashlib.sha512()
hash_.update(self.abspath.encode("utf8"))
return hash_.hexdigest()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was using a hash of the absolute path to the requirements file as the cache key. There's no need: just use the absolute path directly as the cache key.

@@ -75,7 +68,6 @@ def pip_sync_maybe(src_files, args):

def entry_point(): # pragma: nocover
parser = ArgumentParser()
parser.add_argument("-n", "--dry-run", action="store_true")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

--dry-run was unused

Comment on lines -16 to -19
@property
@lru_cache
def path(self):
return self._path
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The path property was unnecessary, can just use a path attribute (as long as no one ever mutates path as that would invalidate the cached abspath and contents_hash properties, but this is really a very small script)

@seanh seanh merged commit d71e242 into main Jul 7, 2022
@seanh seanh deleted the fixups branch July 7, 2022 07:50
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.

1 participant