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

Breaking Change Proposals ('Treasury v3') #287

Open
lokka30 opened this issue Jul 10, 2023 · 0 comments
Open

Breaking Change Proposals ('Treasury v3') #287

lokka30 opened this issue Jul 10, 2023 · 0 comments
Labels
priority: low Low priority status: on hold Development is paused until further notice status: unconfirmed Needs confirmation / validation type: improvement A feature is added or adjusted unassigned developers No maintainers or contributors are assigned to solve this yet unassigned target version No target/ideal project version has been assigned to this yet

Comments

@lokka30
Copy link
Member

lokka30 commented Jul 10, 2023

Nobody is in the mood to make another breaking change to Treasury, though if we ever need to make one in the distant future, it would be best to make as many of these breaking changes all in one go to reduce the burden to implementers.

To avoid clogging up our issue tracker, I'd prefer to group these suggestions here.

Without further ado, let's look at the current proposals. Please add your vote in the comments.


Proposal: Adjust return type of EconomyProvider#registerCurrency from TriState to Boolean

Description

There is no purpose for the third state, FALSE, in the TriState that is returned from EconomyProvider#registerCurrency. TRUE is used for 'it worked', UNSPECIFIED is used for 'it was already registered', and an exception should be thrown in the overlapping CompletableFuture returned if the currency couldn't be registered, leaving zero purpose for the FALSE state.

Therefore, I recommend we simply switch the return type to Boolean so that TriState#TRUE is represented by a true value and TriState#UNSPECIFIED is represented by a false value (so that the boolean returned is a question of whether the currency was not already registered).

Voting

  • Yay: lokka30
  • Nay:

Sketchpad of ideas that need to be described and added above:

  • deleteAccount should return nothing (void), the boolean result false is meaningless since an exception should be thrown if the deletion doesn't go through.

Any further proposals? Add yours below and I'll pop it in here. Your vote counts too - add yours in the comments.

@lokka30 lokka30 added type: improvement A feature is added or adjusted status: on hold Development is paused until further notice priority: low Low priority unassigned target version No target/ideal project version has been assigned to this yet unassigned developers No maintainers or contributors are assigned to solve this yet status: unconfirmed Needs confirmation / validation labels Jul 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low Low priority status: on hold Development is paused until further notice status: unconfirmed Needs confirmation / validation type: improvement A feature is added or adjusted unassigned developers No maintainers or contributors are assigned to solve this yet unassigned target version No target/ideal project version has been assigned to this yet
Projects
None yet
Development

No branches or pull requests

1 participant