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

Show EOL warning in the update section #8821

Merged
merged 1 commit into from
Mar 20, 2018
Merged

Show EOL warning in the update section #8821

merged 1 commit into from
Mar 20, 2018

Conversation

nickvergessen
Copy link
Member

MorrisJobke
MorrisJobke reviewed Mar 14, 2018
View reviewed changes
apps/updatenotification/lib/UpdateChecker.php
@@ -49,6 +49,7 @@ public function getUpdateState(): array {
$result['updateAvailable'] = true;
$result['updateVersion'] = $data['versionstring'];
$result['updaterEnabled'] = $data['autoupdater'] === '1';
$result['versionIsEol'] = $data['eol'] === '1';
Copy link
Member

Choose a reason for hiding this comment

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

Does this also work if eol is not set? (just asking for old updater servers ;))

@MorrisJobke
Copy link
Member

Just one question: we only show this warning if the updater server actually returns something. So we need to return a response with EOL for every old release right?

That also means we should ship the EoL flag for an instance on 11.0.8, that gets a response with 12.0.5, right? So we response with EoL based on the source version and not the destination version. Just want to check back on this here.

@codecov
Copy link

codecov bot commented Mar 14, 2018 •
edited
Loading

Codecov Report

Merging #8821 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #8821      +/-   ##
============================================
- Coverage      51.9%   51.89%   -0.02%     
+ Complexity    25272    25270       -2     
============================================
  Files          1604     1605       +1     
  Lines         94769    94777       +8     
  Branches       1377     1377              
============================================
- Hits          49194    49182      -12     
- Misses        45575    45595      +20
Impacted Files Coverage Δ Complexity Δ
apps/updatenotification/lib/UpdateChecker.php 72.72% <100%> (+1.29%) 8 <0> (ø) ⬇️
lib/private/Updater/VersionCheck.php 85.71% <100%> (+0.34%) 8 <0> (+1) ⬆️
apps/updatenotification/lib/Settings/Admin.php 74.07% <100%> (+0.48%) 12 <0> (+1) ⬆️
lib/public/Share.php 35.29% <0%> (-4.71%) 7% <0%> (+1%)
settings/Controller/GroupsController.php 64.61% <0%> (-3.72%) 9% <0%> (ø)
lib/private/Files/ObjectStore/Swift.php 69.23% <0%> (-3.19%) 9% <0%> (ø)
apps/dav/lib/CalDAV/Activity/Provider/Base.php 76.78% <0%> (-2.58%) 16% <0%> (-3%)
...private/Collaboration/Collaborators/MailPlugin.php 75.72% <0%> (-0.91%) 29% <0%> (-2%)
lib/private/legacy/json.php 9.83% <0%> (-0.88%) 25% <0%> (+2%)
lib/private/AllConfig.php 89.39% <0%> (-0.76%) 51% <0%> (ø)
... and 20 more

nickvergessen
nickvergessen commented Mar 14, 2018
View reviewed changes
lib/private/Updater/VersionCheck.php
@@ -98,6 +98,7 @@ public function check() {
$tmp['url'] = (string)$data->url;
$tmp['web'] = (string)$data->web;
$tmp['autoupdater'] = (string)$data->autoupdater;
$tmp['eol'] = isset($data->eol) ? (string)$data->eol : '0';
Copy link
Member Author

Choose a reason for hiding this comment

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

@MorrisJobke isset check for old servers is here

@nickvergessen
Copy link
Member Author

That also means we should ship the EoL flag for an instance on 11.0.8, that gets a response with 12.0.5, right? So we response with EoL based on the source version and not the destination version. Just want to check back on this here.

Yes, that's what I did on the updater server 👍

@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 14, 2018
@MorrisJobke
Copy link
Member

@nickvergessen Failing unit tests

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Mar 15, 2018
@MorrisJobke MorrisJobke added this to the Nextcloud 14 milestone Mar 15, 2018
@nickvergessen nickvergessen added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Mar 15, 2018
@rullzer
Copy link
Member

rullzer commented Mar 19, 2018

Conflict

@nickvergessen
Show EOL warning in the update section
ec1e039 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
Copy link
Member Author

rebased

@MorrisJobke MorrisJobke merged commit 18994cd into master Mar 20, 2018
@MorrisJobke MorrisJobke deleted the feature/noid/show-eol-warning branch March 20, 2018 21:39
@nickvergessen
Copy link
Member Author

Should we backport this to 11? @MorrisJobke your call.

@MorrisJobke
Copy link
Member

Should we backport this to 11? @MorrisJobke your call.

11 would not make sense, because there will be no future 11 release ;)

But 12 and 13 would be nice

@nickvergessen nickvergessen mentioned this pull request Mar 26, 2018
Merged
@nickvergessen
Copy link
Member Author

Backports in #8986 and #8987

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@rullzer rullzer Awaiting requested review from rullzer

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement feature: install and update
Projects
None yet
Milestone
Nextcloud 14
Development

Successfully merging this pull request may close these issues.
4 participants
@nickvergessen @MorrisJobke @rullzer @skjnldsv