feat/cosmetic: Move Proxy Contracts around #498
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bayological
approved these changes
Aug 19, 2024
There was a problem hiding this comment.
On the fence about having the proxies next to the implementation. I think this would make more sense for the interfaces than proxies since there's never much need to refer back to the proxies, so locating them easily doesn't seem like a problem. On the other hand, we don't have many so not much overcrowding in the folder.
baroooo
reviewed
Aug 21, 2024
baroooo
approved these changes
Aug 21, 2024
bowd
added a commit
that referenced
this pull request
Aug 26, 2024
### Description When we restructured the repo a while ago I moved the proxy contracts to the legacy folder thinking that we might not deploy named proxies in the future, and maybe upgrade to a more modern Proxy implementation, but we have continued to add the proxies inside the `legacy` folder which doesn't feel great as that folder is meant to be removed (and probably can be at this point). So in order to embrace what we're doing I propose collocating the proxy contracts to the `contracts/tokens` directory. And as an addition to that, I propose collocating all proxies with their contracts like Martin also did in the `contracts/oracles` for the new ChainlinkRelayerFactory contract and proxy. So I also moved around the proxies in `contracts/proxies`. This shouldn't really effect anything else. ### Other changes - Renamed stabletokenPSO to stabletokenPHP ### Tested Ran tests, all is well. ### Related issues N/A ### Backwards compatibility YES ### Documentation N/A --------- Co-authored-by: Bayological <6872903+bayological@users.noreply.github.com> Co-authored-by: denviljclarke <60730266+denviljclarke@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
When we restructured the repo a while ago I moved the proxy contracts to the legacy folder thinking that we might not deploy named proxies in the future, and maybe upgrade to a more modern Proxy implementation, but we have continued to add the proxies inside the
legacyfolder which doesn't feel great as that folder is meant to be removed (and probably can be at this point).So in order to embrace what we're doing I propose collocating the proxy contracts to the
contracts/tokensdirectory. And as an addition to that, I propose collocating all proxies with their contracts like Martin also did in thecontracts/oraclesfor the new ChainlinkRelayerFactory contract and proxy. So I also moved around the proxies incontracts/proxies.This shouldn't really effect anything else.
Other changes
Tested
Ran tests, all is well.
Related issues
N/A
Backwards compatibility
YES
Documentation
N/A