Skip to content

Remove unused methods of OC_Response - #8792

Merged
MorrisJobke merged 2 commits into
masterfrom
cleanup-oc_response
Mar 13, 2018
Merged

Remove unused methods of OC_Response#8792
MorrisJobke merged 2 commits into
masterfrom
cleanup-oc_response

Conversation

@MorrisJobke

Copy link
Copy Markdown
Member

Ref #8375 and #7827

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@MorrisJobkeMorrisJobke added 3. to review Waiting for reviews technical debt 🧱 🤔🚀 labels Mar 12, 2018
@MorrisJobkeMorrisJobke added this to the Nextcloud 14 milestone Mar 12, 2018

@ChristophWurstChristophWurst left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good otherwise

Comment threadapps/files_sharing/public.php Outdated
if($token !== '') {
OC_Response::redirect($urlGenerator->linkToRoute($route, array('token' => $token)));
$protocol = \OC::$server->getRequest()->getHttpProtocol();
if ($protocol == 'HTTP/1.1') {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

===?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Just a copy paste and should be replaced soon anyways.

Comment threadapps/files_sharing/public.php Outdated
if ($protocol == 'HTTP/1.1') {
$status = '307 Temporary Redirect';
} else {
$status = '304 Found';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

304 Not Modified is the proper code?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Or 302 Found? :)

Signed-off-by: Morris Jobke <hey@morrisjobke.de>
@codecov

codecovBot commented Mar 13, 2018

Copy link
Copy Markdown

Codecov Report

Merging #8792 into master will decrease coverage by 0.05%.
The diff coverage is 0%.

@@ Coverage Diff @@## master #8792 +/- ##
============================================
- Coverage 51.87% 51.81% -0.06% - Complexity 25358 25379 +21 
============================================
Files 1607 1607 Lines 95101 95087 -14 Branches 1377 1377 ============================================
- Hits 49336 49272 -64 - Misses 45765 45815 +50
Impacted FilesCoverage ΔComplexity Δ
lib/private/legacy/response.php0% <ø> (ø)16 <0> (-20)⬇️
lib/public/Response.php0% <ø> (ø)2 <0> (-7)⬇️
apps/files_sharing/public.php0% <0%> (ø)0 <0> (ø)⬇️
lib/private/Files/ObjectStore/Swift.php0% <0%> (-75%)8% <0%> (ø)
lib/private/Files/ObjectStore/SwiftFactory.php0% <0%> (-52.88%)35% <0%> (ø)
lib/public/DB.php26.66% <0%> (-13.34%)8% <0%> (+3%)
lib/private/Tags.php47.98% <0%> (-4.99%)152% <0%> (+45%)

@MorrisJobke

Copy link
Copy Markdown
MemberAuthor

Thanks @skjnldsv was wrong status code in the initial code. I also fixed the HTTP/1.0 detection ;)

@rullzer

Copy link
Copy Markdown
Member

HTTP 1.0 support needs to die anyway. 1.1 was release in 1997

@skjnldsv

Copy link
Copy Markdown
Member

Cant we return HTTP 418 instead? :D

@MorrisJobke
MorrisJobke merged commit 7488218 into masterMar 13, 2018
@MorrisJobke
MorrisJobke deleted the cleanup-oc_response branch March 13, 2018 08:47
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewstechnical debt🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@MorrisJobke@rullzer@skjnldsv@ChristophWurst