Hi!

Just a few coding standards improvements on vimrc.drush.inc file.

I hope to help.

Thanks.

CommentFileSizeAuthor
#11 test-code-standard-2841000.png50.21 KBhenriquebauer
#10 2841000_10.patch9.59 KBemartoni
#9 interdiff-2-7.txt1.42 KBbenjifisher
#7 coding_standards_for-2841000-7.patch8.38 KBcebasqueira
#2 coding_standards-2841000-2.patch7.6 KBgmaltoni

Comments

gmaltoni created an issue. See original summary.

gmaltoni’s picture

gmaltoni commented
StatusFileSize
new coding_standards-2841000-2.patch7.6 KB

Patch with fixes.

gmaltoni’s picture

gmaltoni commented
Assigned: gmaltoni » Unassigned
cebasqueira’s picture

cebasqueira commented
Title: Coding Standards » Coding Standards for vimrc.drush.inc file
Issue summary: View changes
cebasqueira’s picture

cebasqueira commented
Status: Needs review » Reviewed & tested by the community

+RTBC

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Status: Reviewed & tested by the community » Needs work

Thanks for working on this! It mostly looks good, but I do not see why you want to make a long comment line in the last hunk:

- * Poor substitute for _drush_download_file(), add --no-check-certificate.
- * See http://drupal.org/node/1642444#comment-6742834
+ * Poor substitute for _drush_download_file(), add --no-check-certificate. See http://drupal.org/node/1642444#comment-6742834.

I think the only change I would recommend is changing "See" to "@see" on the second line. https://www.drupal.org/node/1354#see

Probably there is a better solution to the problem with newer versions of drush, but that should not be done as part of this issue.

cebasqueira’s picture

cebasqueira commented
Status: Needs work » Needs review
StatusFileSize
new coding_standards_for-2841000-7.patch8.38 KB

Patch updated!

gmaltoni’s picture

gmaltoni commented
Status: Needs review » Reviewed & tested by the community

I applied the patch and it worked cleanly.

benjifisher’s picture

benjifisher
he/him or they/them
Primary language English
Location Boston area
commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new interdiff-2-7.txt1.42 KB

I have uploaded an interdiff showing the changes from the patch in #2 to the one in #7.

Since this issue is all about coding standards, let's get it right.

-    // See http://drupal.org/node/1642444#comment-6742834.
+    // @See http://drupal.org/node/1642444#comment-6742834.

and

- * Poor substitute for _drush_download_file(), add --no-check-certificate. See http://drupal.org/node/1642444#comment-6742834.
+ * Poor substitute for _drush_download_file(), add --no-check-certificate.
+ *
+ * @See http://drupal.org/node/1642444#comment-6742834.

It should be @see (lower case) with no ending punctuation. Please look at the documentation I referenced in my previous comment.

As long as we are updating these lines, let's also use https:// in the URLs.

emartoni’s picture

emartoni commented
Status: Needs work » Needs review
StatusFileSize
new 2841000_10.patch9.59 KB

Hi all!

Fixed the code standard.
Hope its fine now.

Patch attached.

henriquebauer’s picture

henriquebauer commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new test-code-standard-2841000.png50.21 KB

Looks good!
I have reviewed the 2841000_10.patch and there are no errors, just warnings as you can see in the image.