Part of meta-issue #1518116: [meta] Make Core pass Coder Review

CommentFileSizeAuthor
#18 1650894-coder_review-18.patch32.78 KBfgm
#16 interdiff-1650894-14-16.txt18.03 KBfgm
#16 1650894-coder_review-16.patch34.63 KBfgm
#15 1650894-coder_review-14-report.txt8.86 KBfgm
#14 interdiff-1650894-9-14.txt11.66 KBfgm
#14 1650894-coder_review-14.patch21.33 KBfgm
#9 drupal-make_xmlrpc_module_pass_coder_review-6180146-9.patch23.55 KBsriharsha.uppuluri
#4 drupalcs-xmlrpc.txt1.85 KBtraviscarden
#3 xmlrpc-coder_fixes_part_0f_1518116-1650894-1.patch17.52 KBfotuzlab

Comments

fotuzlab’s picture

fotuzlab commented
Assigned: Unassigned » fotuzlab

I'll do it this weekend. Assigning the issue to myself.

traviscarden’s picture

traviscarden commented

Thanks, @fotuzlab. Don't forget to claim the module on the meta issue, too.

fotuzlab’s picture

fotuzlab commented
Status: Active » Needs review
StatusFileSize
new xmlrpc-coder_fixes_part_0f_1518116-1650894-1.patch17.52 KB

Coder 8 did not work for me. So I used Drupal code sniffer.

Following are the two issues reported by DCS those I skipped.

1. hook_xmlrpc
xmlrpc.api.php
Line 48 | ERROR | No key specified for array entry; first entry specifies key

2. xmlrpc_server
xmlrpc.server.inc
Line 32 | ERROR | No key specified for array entry; first entry specifies key

Attached is the patch for rest of the coder fixes.

traviscarden’s picture

traviscarden commented
Status: Needs review » Postponed
StatusFileSize
new drupalcs-xmlrpc.txt1.85 KB

Thanks, @fotuzlab. Most of this looks pretty good. I'm going to postpone, however, pending resolution of #1539712: [policy, no patch] Coding standards for breaking function calls and language constructs across lines.

While you're waiting, there should be no DocBlock on inherited methods. See http://drupal.org/node/325974. These additions should be removed from the patch. Attached is the drupalcs output after your patch to take care of, too.

fotuzlab’s picture

fotuzlab commented

Thanks Travis. I'll go through the links.
Also the last two errors output by drupalcs are the ones I have mentioned in my last comment. I guess they are to be ignored.

btw, I came across this http://drupal.org/node/1649140 today morning. Assigning it to myself as it relates to this issue.

traviscarden’s picture

traviscarden commented

Can you document your reason for ignoring those drupalcs errors here?

Also, when linking to issues, use this format #1649140: Clean up API docs for xmlrpc module to automatically get this: #1649140: Clean up API docs for xmlrpc module. :)

sphism’s picture

sphism commented
Status: Postponed » Active

We have the go ahead with all these issues again, see #1518116: [meta] Make Core pass Coder Review for more details

sriharsha.uppuluri’s picture

sriharsha.uppuluri commented
Assigned: fotuzlab » sriharsha.uppuluri
Issue summary: View changes

Started Working

sriharsha.uppuluri’s picture

sriharsha.uppuluri commented
Status: Active » Needs review
StatusFileSize
new drupal-make_xmlrpc_module_pass_coder_review-6180146-9.patch23.55 KB

As mentioned in #3 except those remaining are fixed.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
commented
Project: Drupal core » xmlrpc
Version: 8.0.x-dev »
Component: xml-rpc system » Code

After #1285726-48: Remove XML-RPC moving this to the contributed module home for xmlrpc issues.

fgm queued 9: drupal-make_xmlrpc_module_pass_coder_review-6180146-9.patch for re-testing.

drumm’s picture

drumm
he/him
Location NY, US
commented
Version: » 8.x-1.x-dev

Status: Needs review » Needs work

The last submitted patch, 9: drupal-make_xmlrpc_module_pass_coder_review-6180146-9.patch, failed testing.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Needs work » Needs review
StatusFileSize
new 1650894-coder_review-14.patch21.33 KB
new interdiff-1650894-9-14.txt11.66 KB

Rerolled so that patch applies : after that, it will need fixing: drupalcs still reports a ton of warnings with this patch applied.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
StatusFileSize
new 1650894-coder_review-14-report.txt8.86 KB

FWIW, the current coder (CS ) report is attached.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
StatusFileSize
new 1650894-coder_review-16.patch34.63 KB
new interdiff-1650894-14-16.txt18.03 KB

Addressed all CS issues too.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Needs review » Needs work

Need to also address the changes introduced in recent fixed issuesa

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Needs work » Needs review
StatusFileSize
new 1650894-coder_review-18.patch32.78 KB

All issues addressed.

  • fgm committed 30a9d24 on 8.x-1.x 
    Issue #1650894 by fgm, fotuzlab, sriharsha.uppuluri: Make xmlrpc module...
fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Assigned: sriharsha.uppuluri » Unassigned
Status: Needs review » Fixed

Committed to 8.x-1.x. Thanks all for your work.

Status: Fixed » Closed (fixed)

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