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: remove V2 and obsolete references #994

Merged
merged 14 commits into from
Aug 1, 2024
Merged

Conversation

smol-ninja
Copy link
Member

@smol-ninja smol-ninja commented Jul 30, 2024

This PR closes 2nd and 3rd points of Package tethering.

Changelog

  • Remove obsolete references to Core
  • Remove V2
  • SVG contains "Sablier Lockup Linear" etc.
Screenshot 2024-07-30 at 14 49 13

@smol-ninja smol-ninja marked this pull request as draft July 30, 2024 08:54
@smol-ninja smol-ninja changed the title Refactor/remove v2 refactor: remove V2 and use Lockup for the root repo Jul 30, 2024
@andreivladbrg andreivladbrg force-pushed the staging branch 3 times, most recently from 96e3209 to a97e5ae Compare July 30, 2024 10:55
refactor: refer to Lockup contracts

Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>
@smol-ninja smol-ninja changed the title refactor: remove V2 and use Lockup for the root repo refactor: remove V2 and obsolete references Jul 30, 2024
Copy link
Member Author

@smol-ninja smol-ninja left a comment

Choose a reason for hiding this comment

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

@andreivladbrg tagging you for some review.

src/core/SablierNFTDescriptor.sol Outdated Show resolved Hide resolved
src/core/SablierNFTDescriptor.sol Outdated Show resolved Hide resolved
@smol-ninja smol-ninja marked this pull request as ready for review July 30, 2024 13:45
Copy link
Member

@andreivladbrg andreivladbrg left a comment

Choose a reason for hiding this comment

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

Great work!

I see that there are multiple references , especially.MD files, that point to v2-core. Since we will most likely coordinate the merge of staging to main and the renaming of the repo to lockup, I think it's safe to change them now, wdyt?
For example:

repository = "https://github.com/sablier-labs/v2-core"

Feedback below, which I will create a PR with the some of the fixes:

README.md Show resolved Hide resolved
@@ -9,7 +9,7 @@ Parameters

Licensor: Sablier Labs Ltd

Licensed Work: Sablier V2 Core The Licensed Work is (C) 2024 Sablier Labs Ltd
Licensed Work: Sablier Lockup The Licensed Work is (C) 2024 Sablier Labs Ltd

Additional Use Grant: Any uses listed and defined at v2-core-license-grants.sablier.eth
Copy link
Member

Choose a reason for hiding this comment

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

@PaulRBerg should we take the ens domain for lockup-license-grants.sablier.eth ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, we should.

For anything that requires submitting a multisig transaction, you can tag @maxdesalle. He initiates the multisig txs — I just execute them.

package.json Show resolved Hide resolved
script/DeployDeterministicProtocol.s.sol Outdated Show resolved Hide resolved
script/DeployDeterministicProtocol.s.sol Outdated Show resolved Hide resolved
src/core/SablierNFTDescriptor.sol Outdated Show resolved Hide resolved
src/core/libraries/Errors.sol Outdated Show resolved Hide resolved
src/periphery/SablierBatchLockup.sol Outdated Show resolved Hide resolved
src/periphery/interfaces/ISablierBatchLockup.sol Outdated Show resolved Hide resolved
test/core/fork/NFTDescriptor.t.sol Outdated Show resolved Hide resolved
style: center headers
style: improve the asci art
style: remove empty spaces
chore: add todos
@smol-ninja
Copy link
Member Author

I see that there are multiple references , especially.MD files, that point to v2-core. Since we will most likely coordinate the merge of staging to main and the renaming of the repo to lockup, I think it's safe to change them now, wdyt?
For example:

Yes, I was keeping them until we rename the repo but it does not matter. So, I've changed them in 5ad4481.

@smol-ninja
Copy link
Member Author

Added a new commit: 11cd564. LMK if it all looks good now @andreivladbrg.

PS: there are a few open conversations pending your reply.

Copy link
Member

@andreivladbrg andreivladbrg left a comment

Choose a reason for hiding this comment

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

pushed a new commit c6d0051

lmk if it looks good

@smol-ninja smol-ninja merged commit d7162c4 into staging Aug 1, 2024
7 checks passed
@smol-ninja smol-ninja deleted the refactor/remove-v2 branch August 1, 2024 22:59
andreivladbrg added a commit that referenced this pull request Sep 11, 2024
Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>

* refactor: remove V2 from codebase
refactor: refer to Lockup contracts

* fix: make NFTDescriptor compatible with previous versions

* fix: use sablier as variabel name in tokenURI

* chore: update precompiles

* refactor: prefix Sablier in NFT mapsymbol

* refactor(periphery): order alphabetically functions in batch interfaces

style: center headers
style: improve the asci art
style: remove empty spaces
chore: add todos

* test: computing merkle proof for 1 leaf

* refactor: use lockup as variable in NFT Descriptor

* refactor: replace v2-core with lockup in urls

* test: add v1.2.0 to NFTDescriptor_Fork_Test
refactor: use package versions in NFTDescriptor_Fork_Test

* refactor: rename SablierNFTDescriptor to LockupNFTDescriptor

* docs(README): remove periphery reference

* test: add TODO over loadDependencies function

---------

Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>
Co-authored-by: andreivladbrg <[email protected]>
andreivladbrg added a commit that referenced this pull request Oct 8, 2024
Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>

* refactor: remove V2 from codebase
refactor: refer to Lockup contracts

* fix: make NFTDescriptor compatible with previous versions

* fix: use sablier as variabel name in tokenURI

* chore: update precompiles

* refactor: prefix Sablier in NFT mapsymbol

* refactor(periphery): order alphabetically functions in batch interfaces

style: center headers
style: improve the asci art
style: remove empty spaces
chore: add todos

* test: computing merkle proof for 1 leaf

* refactor: use lockup as variable in NFT Descriptor

* refactor: replace v2-core with lockup in urls

* test: add v1.2.0 to NFTDescriptor_Fork_Test
refactor: use package versions in NFTDescriptor_Fork_Test

* refactor: rename SablierNFTDescriptor to LockupNFTDescriptor

* docs(README): remove periphery reference

* test: add TODO over loadDependencies function

---------

Co-authored-by: Andrei Vlad Birgaoanu <[email protected]>
Co-authored-by: andreivladbrg <[email protected]>
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.

3 participants