Problem/Motivation

According to the @file doc standard coding, we should dropped all @file path documentation for classes / interfaces / traits. Plus it would be better to commit refactoring and other non-/functional clean-up fixes as a combined patch.

Proposed resolution

- refactor docblocks
- inspect code for Redirect module
- upload the patch
(Striked through elements will be done in related issue #2722391: General code style cleanup from Redirect inspection)

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#17 related issue.png26.52 KBtduong
#10 interdiff-2-10.txt2.94 KBBambell
#10 general_refactoring-2717103-10.patch11.3 KBBambell
#2 edit_issue_general-2717103-1.patch9.69 KBBambell

Comments

tduong created an issue. See original summary.

Bambell’s picture

Bambell commented
Assigned: Unassigned » Bambell
Status: Active » Needs review
StatusFileSize
new edit_issue_general-2717103-1.patch9.69 KB
edurenye’s picture

edurenye commented 
+++ b/src/Plugin/migrate/process/PathRedirect.php
@@ -44,4 +39,4 @@ class PathRedirect extends ProcessPluginBase {
 
-}
\ No newline at end of file
+}
diff --git a/src/Plugin/migrate/source/PathRedirect.php b/src/Plugin/migrate/source/PathRedirect.php

This is an unrelated change. But for me it's fine this change.

Just try to not add unrelated changes the next time.

tduong’s picture

tduong commented

Yay, good start! :)

This is an unrelated change. But for me it's fine this change.

I think this is something that has been done from git.

Bambell’s picture

Bambell commented

I think this is something that has been done from git.

It's added by PHPStorm actually, I believe. The configuration guide stated that PHPStorm should be configured so that files should end with a new line, so I thought I'd leave it there..

tduong’s picture

tduong commented

True, phpstorm :) Yes, no problem. I was just saying it's not a "fault" if it has been changed. It's fine as it is now :)

tduong’s picture

tduong commented

It is ok, see here: Indenting and Whitespace

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Looks good.

tduong’s picture

tduong commented
Status: Reviewed & tested by the community » Needs work

Please remove also the unused import files.

Then, please change the title of this issue so that here we only care about the basic cleanups, and then create a new issue for the inspect redirect module code stuff.

Bambell’s picture

Bambell commented
StatusFileSize
new general_refactoring-2717103-10.patch11.3 KB
new interdiff-2-10.txt2.94 KB

Here's a patch to remove the unnecessary @file docblocks and the unused imports. I'll rename the issue and open a new one @tduong, sorry for the delay.

Bambell’s picture

Bambell commented
Title: General refactoring from Redirect inspection and removing @file docblocks » General refactoring from Redirect removing @file docblocks
Status: Needs work » Needs review
Bambell’s picture

Bambell commented

The new issue : #2722391: General code style cleanup from Redirect inspection.

tduong’s picture

tduong commented
Status: Needs review » Needs work

No problem :)

Great! Now there is only the IS (issue summary) to be updated and set the followup issue to "Related issues" in this issue, then IMO everything will be fine and ready to be committed.

FYI: you can do [#issue ID] to generate the link to other issues. This is better to be used because then it will get the status-color of the other issue and use it to nicely highlight the link.

Bambell’s picture

Bambell commented
Issue summary: View changes
Bambell’s picture

Bambell commented
Status: Needs work » Needs review
Bambell’s picture

Bambell commented

I updated the IS and added the related issue. Also, thanks for the tip ! I updated those prehistoric <a> tags !

tduong’s picture

tduong commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Related issues: +#2722391: General code style cleanup from Redirect inspection
StatusFileSize
new related issue.png26.52 KB

Cool! RTBC :)

PS: By "Related issues" I meant in the autocomplete form below ( see above the form where you usually upload your patches ;) )

chandeepkhosa’s picture

chandeepkhosa commented
Assigned: Bambell » Unassigned

  • Berdir committed b17d51b on 8.x-1.x authored by Bambell 
    Issue #2717103 by Bambell, tduong: Removing @file docblocks
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Title: General refactoring from Redirect removing @file docblocks » Removing @file docblocks
Status: Reviewed & tested by the community » Fixed

Thanks, committed.

Status: Fixed » Closed (fixed)

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