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

refactor(cache): makeName -> createHash for clarity #355

Merged
merged 2 commits into from
Jun 14, 2022

Conversation

agilgur5
Copy link
Collaborator

Summary

Rename makeName -> createHash and name -> hash in tscache.ts code for clarity

Details

  • I've actually been confused multiple times as to what makeName does when I read through the cache

    • I re-read the code and then am like "oh it's the hash"...
    • so thought renaming it to createHash would make things a lot clearer
      • Name -> Hash
      • make -> create because that's the more common terminology in programming
  • also rename variables that reference makeName's return from name to hash

    • for the same reason around clarity -- this way it's quicker to interpret whenever you see it too
    • not to mention, name can be confusing since we also have id, which is a path that is very similar to a name too
      • and lots of fileNames too
      • so good to disambiguate/differentiate a bit

Misc Notes

Been digging into cache bugs more now that I've fixed a ton of the non-cache ones, so doing a bit of refactoring here as I'm going through it more frequently and in more depth.

- I've actually been confused multiple times as to what `makeName` does when I read through the cache
  - I re-read the code and then am like "oh it's the hash"...
  - so thought renaming it to `createHash` would make things **a lot** clearer
    - `Name` -> `Hash`
    - `make` -> `create` because that's the more common terminology in programming

- also rename variables that reference `makeName`'s return from `name` to `hash`
  - for the same reason around clarity -- this way it's quicker to interpret whenever you see it too
  - not to mention, `name` can be confusing since we also have `id`, which is a path that is very similar to a name too
    - and lots of `fileName`s too
    - so good to disambiguate/differentiate a bit
@agilgur5 agilgur5 added kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs scope: cache Related to the cache labels Jun 11, 2022
@ezolenko
Copy link
Owner

Really it's more like a key, with additional requirement of being filesystem safe, but this works too

@ezolenko ezolenko merged commit 4f93c44 into ezolenko:master Jun 14, 2022
@agilgur5
Copy link
Collaborator Author

agilgur5 commented Jun 14, 2022

Really it's more like a key

Hashes are keys, so I think that terminology works 👍

with additional requirement of being filesystem safe

Ah, that's a good point that I didn't think about, but I guess a sha1 hash is FS-safe, so it works so long as the implementation isn't changed.
Might be good to add a typedoc @returns comment mentioning that for future readers or anyone who may think of changing the hash algorithm (I don't have any plans to do that, but I can imagine someone looking at sha1, saying "let's update that", and not be aware that FS-safe is a required property)

@agilgur5 agilgur5 deleted the refactor-cache-name-hash branch July 2, 2023 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: internal Changes only affect the internals, and _not_ the public API or external-facing docs scope: cache Related to the cache
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants