-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Allow apps to check for a given nextcloud version #556
Conversation
@nickvergessen, thanks for your PR! By analyzing the annotation information on this pull request, we identified @DeepDiver1975, @BernhardPosselt, @MorrisJobke and @Xenopathic to be potential reviewers |
I don't think the |
well that is why I made it deprecated, it doesn't cause any error on validation or whatsoever, just says that the value is not required anymore |
great idea. 👍 please backport! |
@@ -9,7 +9,7 @@ | |||
<author>Nextcloud</author> | |||
<version>1.1.0</version> | |||
<dependencies> | |||
<owncloud min-version="9.2" max-version="9.2" /> |
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.
@karlitschek isn't this breaking the compatibility we have been trying to keep up?
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.
Well the next cloud version 10 kinda broke it anyways
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.
What I mean is that versioning is totally fcked up at the moment
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.
@DeepDiver1975 both are supported, and our shipped "core apps" are not required to work on ownCloud anyway? I mean switching our files app with yours or the other way around makes little sense to me?
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.
@BernhardPosselt Well 10 is only the human readable version number. internally it is 9.1 exactly because we don't want to break apps. :-)
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.
Right I mean the version started a new versioning schema which will break compatibility in the next version (11 will be 11 internally, correct?)
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.
@DeepDiver1975 Hmm. Interesting. This tags a re optional. So I'm not sure this would break anything. But we are happy to discuss if we can improve something. call?
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.
Right I mean the version started a new versioning schema which will break compatibility in the next version (11 will be 11 internally, correct?)
I don't think so...
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.
@DeepDiver1975 Hmm. Interesting. This tags a re optional. So I'm not sure this would break anything. But we are happy to discuss if we can improve something. call?
Looks like soled 😉
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.
Thanks for keeping up the discussion between the projects 🚀
e292868
to
9c3dea2
Compare
Updated to only translate the version, sorry for the misunderstanding, confusions and troubles |
9c3dea2
to
0fcc39c
Compare
👍 |
@nickvergessen Could you create the backport PR to stable10? |
@MorrisJobke no, will be on vacation until it's too late. |
Stable10: #757 |
Allow apps to check for a given nextcloud version
Translates the displayed server version in dependency errors
10.0*
maps to9.1*
11.0*
maps to9.2*
@MorrisJobke @LukasReschke
@karlitschek I'd like to backport this down to 9, so new apps are displayed/denied with the correct information.