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

Update version on master #1170

Merged
merged 1 commit into from
Apr 15, 2022
Merged

Update version on master #1170

merged 1 commit into from
Apr 15, 2022

Conversation

nickvergessen
Copy link
Member

Signed-off-by: Joas Schilling coding@schilljs.com

Signed-off-by: Joas Schilling <coding@schilljs.com>
@jotoeri
Copy link
Member

jotoeri commented Apr 14, 2022

We'll need it for the tests of course.
@skjnldsv How does that work with your new release-workflow? Needs most probably manual adaption before, then? 🤔

@skjnldsv
Copy link
Member

Yeah, it's a good question. I'm not sure we should have 25 here. Or else we'll have to edit to 24 before each release. Not very efficient 🙈

But thinking about it, as long as 25 is not released, that doesn't change anything, as the only people that will install this app on 25 would be devs. So we could indeed keep this as 22-25, but when we're reaching 25 release date, we'll have to release a fully tested version of Forms on 25 anyway. So that doesn't change anything in then end.

Yeah, I think we should merge it.

@nickvergessen
Copy link
Member Author

nickvergessen commented Apr 15, 2022

Yeah that is also my conclusion. The problem with not bumping the requirement is that you can't dev/test with master anymore.
I used to manually change the info.xml before packaging and then reverting it, but since we now build via ci you would need to commit it all the time.

@jotoeri jotoeri merged commit 4b23e63 into master Apr 15, 2022
@jotoeri jotoeri deleted the update-master-version branch April 15, 2022 07:42
@nursoda
Copy link

nursoda commented Apr 19, 2022

Interesting…but somewhat irritating. You assume –in case NC25 does have incompatible requirements– that

  • you will timely release a bugfix version that reverts info.xml to 24.x
  • admins and users only update their apps from the Nextcloud app store (and not from local caches)

While I agree that these assumptions hold true for most installations, it doesn't make the uneasiness go away that the human readable semantics is misleading. As admin, I also don't expect max-version downgrades. I'd propose to exclude the max-version checks if run on non-released nightly master/CI build. This obviously should not be discussed on an app's pull request…

@jotoeri
Copy link
Member

jotoeri commented Apr 24, 2022

I'd propose to exclude the max-version checks if run on non-released nightly master/CI build

Interesting point... @nickvergessen Is there any way of using the possibliity to 'activate untested app' via occ? 🤔

@nickvergessen
Copy link
Member Author

occ app:enable --force should do

@jotoeri
Copy link
Member

jotoeri commented Apr 24, 2022

Interesting... We have that already on the phpunit tests 🙈. So probably we wouldn't even need to change the serverversion for running the tests. 🤔

Edit: Yep, would work without. https://github.com/nextcloud/forms/actions/runs/2216859426

@nickvergessen
Copy link
Member Author

Yeah I was not aware of the --force until recently. It might be the solution for all the cases in tests of server and when other apps are needed to test, as well as in multi apps themselves.

So a potentially new way of handling them would be:

  1. Always develop with info.xml being set to supported versions only
  2. Master of Nextcloud can be tested/developed against with the --force option
  3. Once nearing a new major beta/rc/final of server, the version is added to info.xml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants