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

Nextcloud 13 is not compatible with newer than php 7.2 #6830

Merged
merged 1 commit into from
Dec 12, 2017
Merged

Nextcloud 13 is not compatible with newer than php 7.2 #6830

merged 1 commit into from
Dec 12, 2017

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented Oct 13, 2017

Easy one.
Just so we don't forget and have people try NC13 with a to new version of php.

@rullzer rullzer added 3. to review Waiting for reviews enhancement labels Oct 13, 2017
@rullzer rullzer added this to the Nextcloud 13 milestone Oct 13, 2017
@nickvergessen
Copy link
Member

There should be more? remote.php? ocs/?

@codecov
Copy link

codecov bot commented Oct 13, 2017 •
edited
Loading

Codecov Report

Merging #6830 into master will decrease coverage by <.01%.
The diff coverage is 0%.

@@             Coverage Diff              @@
##             master    #6830      +/-   ##
============================================
- Coverage     51.11%   51.11%   -0.01%     
  Complexity    24900    24900              
============================================
  Files          1600     1601       +1     
  Lines         94763    94772       +9     
  Branches       1367     1367              
============================================
- Hits          48439    48438       -1     
- Misses        46324    46334      +10
Impacted Files Coverage Δ Complexity Δ
ocs/v1.php 0% <0%> (ø) 0 <0> (ø) ⬇️
status.php 0% <0%> (ø) 0 <0> (ø) ⬇️
cron.php 0% <0%> (ø) 0 <0> (ø) ⬇️
index.php 0% <0%> (ø) 0 <0> (ø) ⬇️
ocs/providers.php 0% <0%> (ø) 0 <0> (ø) ⬇️
lib/versioncheck.php 0% <0%> (ø) 0 <0> (?)
public.php 0% <0%> (ø) 0 <0> (ø) ⬇️
remote.php 0% <0%> (ø) 0 <0> (ø) ⬇️
console.php 0% <0%> (ø) 0 <0> (ø) ⬇️
apps/files_trashbin/lib/Trashbin.php 72.28% <0%> (-0.25%) 136% <0%> (ø)
... and 2 more

@rullzer
Copy link
Member Author

rullzer commented Oct 13, 2017

@nickvergessen they include base.php

@ChristophWurst
Copy link
Member

There should be more? remote.php? ocs/?

Based on what/where we currently check for a min-version, also console.php should have a check: https://github.com/nextcloud/server/search?utf8=%E2%9C%93&q=PHP_VERSION

@nickvergessen
Copy link
Member

they include base.php

yes @rullzer but I don't see any checking there either?

@LukasReschke
Copy link
Member

There should be more? remote.php? ocs/?

If we do that we need to also add a 500 error status code. Otherwise our sync clients may get a 200 and sync down the text error message … 😱

@nickvergessen
Copy link
Member

Well the same happens when there is another language based error?
That's exactly why I would like to "stop" these pages as well

@MorrisJobke
Copy link
Member

@rullzer Could you make this a 500 page?

@MorrisJobke MorrisJobke added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Nov 9, 2017
@MorrisJobke
Copy link
Member

Let's move this to 14 and maybe backport it.

@MorrisJobke MorrisJobke modified the milestones: Nextcloud 13, Nextcloud 14 Dec 8, 2017
@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Dec 12, 2017
MorrisJobke
MorrisJobke approved these changes Dec 12, 2017
View reviewed changes
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works 👍 It now also serves a 500 status code

@MorrisJobke
Copy link
Member

CI failure should be fixed by #7458

@rullzer rullzer modified the milestones: Nextcloud 14, Nextcloud 13 Dec 12, 2017
@rullzer
Copy link
Member Author

rullzer commented Dec 12, 2017

Lets get this into 13 then. So it is there from the start.

@blizzz
Copy link
Member

blizzz commented Dec 12, 2017

Should also be documented somewhere, docs or release notes, or both

@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 Dec 12, 2017
@MorrisJobke
Copy link
Member

Should also be documented somewhere, docs or release notes, or both

Already is ;)

@MorrisJobke
Copy link
Member

Let me rebase - to check if CI is green.

@MorrisJobke MorrisJobke mentioned this pull request Dec 12, 2017
Merged
28 tasks
@rullzer
Nextcloud 13 is not compatible with newer than php 7.2
b8af7ee 
Just to avoid users from trying this with a to new (untested) php version

* Moved the check logic to 1 place
* All directly callable scripts just require this on top
* exit hard (-1) so we know scripts won't continue
* Return status 500 so no sync clients will try fancy stuff

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@MorrisJobke
Copy link
Member

Failure is due to a connection issue while cloning -> merge

@MorrisJobke MorrisJobke merged commit 134bd51 into master Dec 12, 2017
@MorrisJobke MorrisJobke deleted the nc13_no_later_72 branch December 12, 2017 10:27
@MorrisJobke MorrisJobke mentioned this pull request Feb 7, 2018
Closed
@rullzer rullzer mentioned this pull request Feb 7, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blizzz blizzz blizzz approved these changes

@MorrisJobke MorrisJobke MorrisJobke approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@LukasReschke LukasReschke Awaiting requested review from LukasReschke

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

@danxuliu danxuliu Awaiting requested review from danxuliu

@icewind1991 icewind1991 Awaiting requested review from icewind1991

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

Successfully merging this pull request may close these issues.
6 participants
@rullzer @nickvergessen @ChristophWurst @LukasReschke @MorrisJobke @blizzz