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.

Sign up for GitHub

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

Jump to bottom

on installation save channel to config.php if not stable #34136

Merged
merged 2 commits into from
Oct 21, 2022
Merged

on installation save channel to config.php if not stable #34136

merged 2 commits into from
Oct 21, 2022

Conversation

blizzz
Copy link
Member

@blizzz blizzz commented Sep 19, 2022

  • the default channel to the NC server is what is provided in /version.php unless it is overridden in config.php
  • the default channel to the NC Updater however is 'stable'
  • this resultant in inconsistent results and confusing admin experience
  • therefore "stable" is considered default and other channels are being written to config.php now upon installation

This seems to be more straight forward than to maintaining the same strategy of determining a default on the Updater (and forgetting about it).

Fixes nextcloud/updater#436

@blizzz blizzz added bug 3. to review Waiting for reviews labels Sep 19, 2022
@blizzz blizzz added this to the Nextcloud 25 milestone Sep 19, 2022
@blizzz blizzz requested review from PVince81, come-nc, a team and skjnldsv and removed request for a team September 19, 2022 11:56
@blizzz blizzz marked this pull request as ready for review September 19, 2022 11:56
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 19, 2022
View reviewed changes
lib/private/Setup.php Fixed Show fixed Hide fixed
PVince81
PVince81 approved these changes Sep 19, 2022
View reviewed changes
Copy link
Member

@PVince81 PVince81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

another alternative would be to make the updater also read the channel from version.php, I believe this information is already read anyway ?

come-nc
come-nc reviewed Sep 19, 2022
View reviewed changes
lib/private/Setup.php Outdated Show resolved Hide resolved
@come-nc
Copy link
Contributor

come-nc commented Sep 19, 2022

(did not approve because I do not understand it…)

@blizzz
Copy link
Member Author

blizzz commented Sep 19, 2022

another alternative would be to make the updater also read the channel from version.php, I believe this information is already read anyway ?

as said in my opening ^:

This [approach] seems to be more straight forward than to maintaining the same strategy of determining a default on the Updater (and forgetting about it).

@blizzz blizzz force-pushed the fix/default-channel branch from 0fc5d7d to 355a483 Compare September 19, 2022 12:56
@blizzz
Copy link
Member Author

blizzz commented Sep 19, 2022

(did not approve because I do not understand it…)

The updater defaults to the "stable" channel, if none is set otherwise: https://github.com/nextcloud/updater/blob/master/lib/Updater.php#L123-L125
But Nextcloud server, defaults to what is written in version.php.

Now when you install the beta, and a new beta release is available, Nextcloud will notify you about it, but the updater would tell you that there is not update available for beta are only available on beta channel.

@blizzz
Copy link
Member Author

blizzz commented Sep 19, 2022

I don't know how to make psalm shut up. composer run psalm:update-baseline ends in errors.

@blizzz blizzz mentioned this pull request Sep 19, 2022
Merged
@come-nc
Copy link
Contributor

come-nc commented Sep 19, 2022

I don't know how to make psalm shut up. composer run psalm:update-baseline ends in errors.

composer run psalm:update-baseline is for removing errors from baseline, not adding them.
You need to run composer run psalm -- --set-baseline=tests/psalm-baseline.xml iirc

This was referenced Sep 20, 2022
Closed
Merged
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@blizzz
Copy link
Member Author

blizzz commented Oct 21, 2022

/rebase

@blizzz
on installation save channel to config.php if not stable
403923d 
- the default channel to the NC server is what is provided in /version.php
  unless it is overridden in config.php
- the default channel to the NC Updater however is 'stable'
- this resultant in inconsistent results and confusing admin experience
- therefore "stable" is considered default and other channels are being
  written to config.php now upon installation

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz
silence psalm
2ef9826 
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzz blizzz force-pushed the fix/default-channel branch from 8819dfc to 2ef9826 Compare October 21, 2022 13:35
@blizzz blizzz added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 21, 2022
@blizzz
Copy link
Member Author

blizzz commented Oct 21, 2022

/backport to stable25

@blizzz
Copy link
Member Author

blizzz commented Oct 21, 2022

/backport to stable24

@blizzz
Copy link
Member Author

blizzz commented Oct 21, 2022

/backport to stable23

@blizzz blizzz merged commit 294a00d into master Oct 21, 2022
@blizzz blizzz deleted the fix/default-channel branch October 21, 2022 15:14
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Oct 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@CarlSchwan CarlSchwan CarlSchwan approved these changes

@PVince81 PVince81 PVince81 approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.

Updating from 25.0.0 beta5 to beta6 shows no update
5 participants
@blizzz @come-nc @PVince81 @CarlSchwan @skjnldsv