Problem/Motivation

After this #2803661: Removed deprecated method calls from code base is committed, we can continue cleaning up some code, e.g. some construct docblock have copy/paste leftovers, $request using a certain import (RequestStack --> Request), inject methods for example in RevisionOverviewForm, ...

Proposed resolution

- clean these classes
- upload the patch

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#8 sanitise_diff_construct-2809415-8.patch20.72 KBtduong
#8 interdiff-2809415-5-8.txt2.24 KBtduong
#5 sanitise_diff_construct-2809415-5.patch21.8 KBtduong
#2 sanitise_diff_construct-2809415-2.patch15.5 KBtduong

Comments

tduong created an issue. See original summary.

tduong’s picture

tduong commented
Assigned: Unassigned » tduong
StatusFileSize
new sanitise_diff_construct-2809415-2.patch15.5 KB

First cleanup patch.

tduong’s picture

tduong commented
Status: Active » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work 
+++ b/src/Form/RevisionOverviewForm.php
@@ -113,7 +135,9 @@ class RevisionOverviewForm extends FormBase {
+      $container->get('request_stack')->getCurrentRequest()

never inject the request, always inject the request stack and access the request when you need it. this defeats the purpose of having a stack.

There might be multiple requests and accessing the request stack ensurs you get the currently active one.

tduong’s picture

tduong commented
Status: Needs work » Needs review
StatusFileSize
new sanitise_diff_construct-2809415-5.patch21.8 KB

Rerolled/reverted some $request back to $requestStack, refactor some leftover from the parent cleanup issue (entity type manager) and sanitise other diff __construct()/properties docblocks.

No interdiff possible.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Priority: Minor » Major

Promoting to major as it changes the API (request => request_stack).

Status: Needs review » Needs work

The last submitted patch, 5: sanitise_diff_construct-2809415-5.patch, failed testing.

tduong’s picture

tduong commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-2809415-5-8.txt2.24 KB
new sanitise_diff_construct-2809415-8.patch20.72 KB

Rerolled and solved conflict. Manually edited the interdiff.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Fixed

Committed, thx.

  • miro_dietiker committed 1c8d2b2 on 8.x-1.x authored by tduong 
    Issue #2809415 by tduong: Sanitise diff construct variables/instances...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.