farcasterxyz / contracts

Implementation of the Farcaster contracts
https://www.farcaster.xyz/
MIT License
340 stars 122 forks source link

refactor(StorageRegistry): rename StorageRent to StorageRegistry #299

Closed varunsrin closed 1 year ago

varunsrin commented 1 year ago

Motivation

The contract was inconsistently called Storage and StorageRent in different places. Renaming it to StorageRegistry aligns with the naming convention used in other contracts and clarifies its purpose.

Change Summary

Merge Checklist


PR-Codex overview

This PR focuses on renaming the StorageRent contract to StorageRegistry. The notable changes include:

The following files were skipped due to too many changes: script/Deploy.s.sol, docs/docs.md, src/Bundler.sol, test/Deploy/Deploy.t.sol, test/Bundler/Bundler.t.sol, test/StorageRent/StorageRent.t.sol

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

horsefacts commented 1 year ago

@varunsrin nitpick: I think just naming this Storage is the best suggestion so far. This was originally named StorageRegistry, but I swapped Registry for Rent since unlike the "registry" contracts, it doesn't expose storage allocations on chain, just through events. "Registry" implies to me that there's something to look up on chain, which is the case for IdRegistry and KeyRegistry.

Not a hill I will die on, though. 😄

varunsrin commented 1 year ago

That's fair, will change

On Mon, Jul 31, 2023 at 4:23 PM horsefacts @.***> wrote:

@.**** approved this pull request.

— Reply to this email directly, view it on GitHub https://github.com/farcasterxyz/contracts/pull/299#pullrequestreview-1555945577, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEBD4SCQCDFCC73PBSQUMLXTA5AVANCNFSM6AAAAAA265VPFE . You are receiving this because you were mentioned.Message ID: @.***>

varunsrin commented 1 year ago

@horsefacts the one annoying thing about Storage vs StorageRegistry is that storage isn't a valid variable name, so lets keep it Registry for now.

github-actions[bot] commented 1 year ago

Coverage after merging varunsrin/rename-storage-rent-admin into main will be

98.70%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   Bundler.sol100%100%100%100%
   FnameResolver.sol100%100%100%100%
   IdRegistry.sol100%100%100%100%
   KeyRegistry.sol100%100%100%100%
   StorageRegistry.sol100%100%100%100%
src/lib
   Signatures.sol100%100%100%100%
   TransferHelper.sol0%0%0%0%15, 15, 15, 25–26, 26, 26
   TrustedCaller.sol100%100%100%100%