-
Notifications
You must be signed in to change notification settings - Fork 16
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
Proof of concept: MastodonMinServerVersion annotation #335
base: master
Are you sure you want to change the base?
Conversation
0e19827
to
29e379b
Compare
"""^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)""" + | ||
"""(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)""" + | ||
"""(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?""" + | ||
"""(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?${'$'}""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This might need to be a bit more lenient, seeing as Mastodon doesn’t follow semantic versioning to the letter at least when it comes to release candidates, as can be seen in this example JSON:
"version": "4.0.0rc1", |
They only switched to a more SemVer-friendly variant starting with 4.2.0-beta3 in mastodon/mastodon#26653:
https://github.com/mastodon/mastodon/blob/v4.2.0-beta3/lib/mastodon/version.rb#L36-L38
I agree with your self-criticism. Annotations are great if they help remove boilerplate code, or at least having to write it ourselves - but having to add a call to The only thing I'd like to see for the moment is an option to disable version checks when building the MastodonClient (or reduce them to less than an exception), so that library users at least have the choice to use BigBone with all those platforms that offer a Mastodon-compatible API while using very different versioning, for example Pixelfed, GotoSocial, Hometown, and probably many more. Long-term, I believe having a proper way to support different platforms would be useful (see #226). |
Description
Our Wiki mentions:
So far, that was not at all the case. But I like the idea, so I wanted to implement a proof of concept which is how this PR should be seen: Not a mergeable change set, but grounds for discussion.
I have added the annotation and additional functions to check if a called method’s min version is higher than or equal to the known instance version.
I’m currently not too happy with the
InstanceMethods::getInstance.requireMinVersion(client)
call and that would be my main criticism for the current solution. My knowledge regarding annotations, reflection, or annotation-based code is very much in its infancy so I’m sure there are better solutions that I’d still need to learn about.ksp
might be an option to auto-create the boilerplate code so that we only need to add the annotation and then ksp handles the rest for us and automatically adds code similar to what I’ve added forgetInstance
as an example for now.Before I continue with this, I’d like to find out what you think of this. Is this a good way forward? Would you like a different approach?
Type of Change
(Keep the one that applies, remove the rest)
Breaking change
Methods annotated with
@MastodonMinServerVersion
may now throw aBigBoneVersionException
which leads to errors from Java callers if they’re not explicitly handling that exception (or its parentException
).How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Mandatory Checklist
gradle check
and there were no errors reported