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.

Sign up for GitHub

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

Jump to bottom

MET-126: Added new option: CollectionDetails::V2 #96

Merged
merged 4 commits into from
Feb 21, 2024
Merged

MET-126: Added new option: CollectionDetails::V2 #96

merged 4 commits into from
Feb 21, 2024

Conversation

kstepanovdev
Copy link
Contributor

@kstepanovdev kstepanovdev commented Feb 13, 2024 •
edited
Loading

Although the new CollectionDetails variant has been added with no additional logic on top, the changes happen to be relatively big. I'd rather completely remove the V1 option with its related methods (or even whole files like bubblegum_set_collection_size), however, I'm worried that would destroy backward compatibility.

@kstepanovdev kstepanovdev added the enhancement New feature or request label Feb 13, 2024
@kstepanovdev kstepanovdev self-assigned this Feb 13, 2024
@kstepanovdev kstepanovdev reopened this Feb 13, 2024
@kstepanovdev kstepanovdev marked this pull request as draft February 13, 2024 10:01
@kstepanovdev kstepanovdev force-pushed the new_collection_details_variant branch from faf4fdc to a1f3e95 Compare February 19, 2024 14:26
@kstepanovdev kstepanovdev marked this pull request as ready for review February 19, 2024 14:38
blockiosaurus
blockiosaurus approved these changes Feb 19, 2024
View reviewed changes
Copy link
Contributor

@blockiosaurus blockiosaurus left a comment

Choose a reason for hiding this comment

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

This looks great! One thing to note is that we usually try and split style changes out from feature changes to make reviews easier. Not sure if that was your editor or not, though!

@kstepanovdev
Copy link
Contributor Author

This looks great! One thing to note is that we usually try and split style changes out from feature changes to make reviews easier. Not sure if that was your editor or not, though!

Thanks.
tbf, I've done it manually. Should I roll back "style" changes?

@blockiosaurus
Copy link
Contributor

This looks great! One thing to note is that we usually try and split style changes out from feature changes to make reviews easier. Not sure if that was your editor or not, though!

Thanks. tbf, I've done it manually. Should I roll back "style" changes?

Nope you're good to merge. Just a comment for the future.

@kstepanovdev kstepanovdev merged commit 300405f into main Feb 21, 2024
9 checks passed
@kstepanovdev kstepanovdev deleted the new_collection_details_variant branch February 21, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blockiosaurus blockiosaurus blockiosaurus approved these changes

@danenbm danenbm Awaiting requested review from danenbm

Assignees

@kstepanovdev kstepanovdev

Labels
enhancement New feature or request
Projects
Metaplex
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kstepanovdev @blockiosaurus