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

Changes to the EconomyAPIVersion #46

Closed
MrNemo64 opened this issue Nov 12, 2021 · 4 comments · Fixed by #52
Closed

Changes to the EconomyAPIVersion #46

MrNemo64 opened this issue Nov 12, 2021 · 4 comments · Fixed by #52
Assignees
Labels
help wanted Requires contributor help to solve. priority: high High priority status: confirmed Approved / validated thoughts wanted Thoughts from other developers would be appreciated on this issue. type: improvement A feature is added or adjusted

Comments

@MrNemo64
Copy link
Contributor

Right now the EconomyAPIVersion class is really simple: Identifyes mayor versions with a number, not more not less. I think this should change. I propose some changes

  1. Change the use of the @since tag, instead of @since v1.0.0 we could use @since {@link me.lokka30.treasury.api.economy.misc.EconomyAPIVersion#VERSION_1 v1.0.0} in the documentation shows the same but it provides a link to the enum version
  2. Change enum entryes naming, instead of VERSION_1 we could use v1_0_0
  3. Specify an order. All enums implement the Comparable<> interface with the default logic being indexes. Since this logic doesn't necesarily apply to us it would make sense to specify out comparing logic
  4. Change the number system. Using integers to know what version it is means that we can only represent big updates, we could have an array of ints/shorts/bytes (probably shorts is enough for all versions we may release)
  5. (Optional) in de documentation of each entry list the changes of that version

I can make a class with this features so you see what I mean

@lokka30
Copy link
Member

lokka30 commented Nov 13, 2021

Change the use of the @SInCE tag, instead of @SInCE v1.0.0 we could use @SInCE {@link me.lokka30.treasury.api.economy.misc.EconomyAPIVersion#VERSION_1 v1.0.0} in the documentation shows the same but it provides a link to the enum version

Great idea.


Change enum entries naming, instead of VERSION_1 we could use v1_0_0

Change the number system. Using integers to know what version it is means that we can only represent big updates, we could have an array of ints/shorts/bytes (probably shorts is enough for all versions we may release)

I agree. Although I am unsure whether we should have the minor version (third digit).


Specify an order. All enums implement the Comparable<> interface with the default logic being indexes. Since this logic doesn't necessarily apply to us it would make sense to specify out comparing logic

I think Java automatically orders the constants by the hierarchy in which they are defined in the code with their own index or something like that.


(Optional) in de documentation of each entry list the changes of that version

We could solve this by linking to a changelog file, better than storing it in the code :)

@Jikoo
Copy link
Collaborator

Jikoo commented Nov 13, 2021

Change enum entries naming, instead of VERSION_1 we could use v1_0_0

Change the number system. Using integers to know what version it is means that we can only represent big updates, we could have an array of ints/shorts/bytes (probably shorts is enough for all versions we may release)

I agree. Although I am unsure whether we should have the minor version (third digit).

In semantic versioning, third digit is patch. I think API versioning should be major/minor only - Major would be breaking changes, minor would be new, non-breaking features. Though it is possible, particularly with the continuously blurring lines between interfaces and implementations, patches for bugfixes should largely just be on the plugin end.

That is, however, a reason for multiple repos - with multiple modules, it doesn't make sense to increment the API's version in maven for a purely plugin-side patch. If the API needs a change, certainly, but otherwise it's just a hassle for developers - they'll have to check each update for compatibility when they could just follow the API for changes instead. It feels weird (to me at least, it's certainly done) to have multiple separate projects in one combined repo.

Specify an order. All enums implement the Comparable<> interface with the default logic being indexes. Since this logic doesn't necessarily apply to us it would make sense to specify out comparing logic

I think Java automatically orders the constants by the hierarchy in which they are defined in the code with their own index or something like that.

Correct, Enum#ordinal follows declaration order. As long as versions are declared in order, comparisons should function as expected. I wouldn't dislike a more robust or user-friendly comparison method though, ordering is a pretty fragile way to set things up.

@MrNemo64
Copy link
Contributor Author

MrNemo64 commented Nov 13, 2021

Change enum entries naming, instead of VERSION_1 we could use v1_0_0

Change the number system. Using integers to know what version it is means that we can only represent big updates, we could have an array of ints/shorts/bytes (probably shorts is enough for all versions we may release)

I agree. Although I am unsure whether we should have the minor version (third digit).

In semantic versioning, third digit is patch. I think API versioning should be major/minor only - Major would be breaking changes, minor would be new, non-breaking features. Though it is possible, particularly with the continuously blurring lines between interfaces and implementations, patches for bugfixes should largely just be on the plugin end.

That is, however, a reason for multiple repos - with multiple modules, it doesn't make sense to increment the API's version in maven for a purely plugin-side patch. If the API needs a change, certainly, but otherwise it's just a hassle for developers - they'll have to check each update for compatibility when they could just follow the API for changes instead. It feels weird (to me at least, it's certainly done) to have multiple separate projects in one combined repo.

You're right. Maybe adding a new enum entry every time a bug is solved is a bit overkill

Specify an order. All enums implement the Comparable<> interface with the default logic being indexes. Since this logic doesn't necessarily apply to us it would make sense to specify out comparing logic

I think Java automatically orders the constants by the hierarchy in which they are defined in the code with their own index or something like that.

That's my problem, letting the order in whitch the entryes are declarated decide what version is newer feels too weak.
My idea is to have in each entry an array of ints/shorts/bytes and maybe a string and have methods like isNewerThan or isOlderThan etc that use that array to know whitch one is older

(Optional) in de documentation of each entry list the changes of that version

We could solve this by linking to a changelog file, better than storing it in the code :)

Probably better

@lokka30

This comment has been minimized.

@MrIvanPlays MrIvanPlays mentioned this issue Dec 2, 2021
@lokka30 lokka30 assigned lokka30 and unassigned lokka30 Dec 4, 2021
@lokka30 lokka30 added status: confirmed Approved / validated thoughts wanted Thoughts from other developers would be appreciated on this issue. type: improvement A feature is added or adjusted help wanted Requires contributor help to solve. priority: high High priority labels Dec 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Requires contributor help to solve. priority: high High priority status: confirmed Approved / validated thoughts wanted Thoughts from other developers would be appreciated on this issue. type: improvement A feature is added or adjusted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants