Followup to #2729829: Coding Standard fixes - Round 1

See http://pareview.sh/pareview/httpsgitdrupalorgprojectsharethisgit-7x-2x

CommentFileSizeAuthor
#10 coding_standard_fixes-2766919-9.patch5.42 KBmanjit.singh
#7 coding_standard_fixes-2766919-4.patch5.16 KBmanjit.singh
#4 coding_standard_fixes-2766919-3.patch5.14 KBmanjit.singh
#2 coding_standard_fixes-2766919-2.patch52.54 KByogeshmpawar

Comments

Yogesh Pawar created an issue. See original summary.

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location Pune(MH), INDIA 🇮🇳
commented
StatusFileSize
new coding_standard_fixes-2766919-2.patch52.54 KB

Updated patch for the coding standard fixes

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Status: Needs review » Needs work

Thanks for the patch. Patch will not apply.
pLease do the git pull origin 7.x-2.x on you local and after resolving merge conflicts post the patch then

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Status: Needs work » Needs review
StatusFileSize
new coding_standard_fixes-2766919-3.patch5.14 KB

Hey, I think most of the coding standard have already fixed in 7.x-2.x. But few of them were pending. So i have updated the patch. Please review it.

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Status: Needs review » Needs work

Thanks for the patch!

  1. +++ b/ShareThisForm.css
@@ -1,3 +1,8 @@
+/**

    +1 

  2. +++ b/sharethis.module
@@ -7,7 +7,6 @@
  * Displays help and module information.

    Remove this description line in comment as well. Its not needed.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented

Did not understand the changes that you have mentioned in ShareThisForm.css Can you please elaborate ?

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Status: Needs work » Needs review
StatusFileSize
new coding_standard_fixes-2766919-4.patch5.16 KB

In the meantime i have updated the patch.

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Status: Needs review » Needs work

#6
That was +1 from my side as well. Nice addition.

one more thing
http://cgit.drupalcode.org/sharethis/tree/ShareThisForm.js#n116
The variable i is not defined before usage. so it will show warnings. As we are on this.So define this variable as well.
Rest looks good to me.

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Issue summary: View changes
manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
StatusFileSize
new coding_standard_fixes-2766919-9.patch5.42 KB

Nice catch :)

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented
Status: Needs work » Needs review
navneet0693’s picture

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

Patch is working fine, clear to go :)

naveenvalecha’s picture

naveenvalecha
He/Him
commented
Priority: Normal » Minor
Status: Reviewed & tested by the community » Fixed

Thanks! Committed and pushed to 7.x-2.x Will be included in next release.

manjit.singh’s picture

manjit.singh
Primary language Hindi
Location Chandigarh
commented

Thanks a lot Nav (een & neet) ;)

Status: Fixed » Closed (fixed)

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