Skip to content

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

Merged
ezolenko merged 2 commits into
ezolenko:masterfrom
agilgur5:refactor-cache-name-hash
Jun 14, 2022
Merged

refactor(cache): makeName -> createHash for clarity#355
ezolenko merged 2 commits into
ezolenko:masterfrom
agilgur5:refactor-cache-name-hash

Conversation

@agilgur5

Copy link
Copy Markdown
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
Copy Markdown
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

agilgur5 commented Jun 14, 2022

Copy link
Copy Markdown
Collaborator Author

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