There are some minor errors in the code base

CommentFileSizeAuthor
#12 edit_fix_coding_standards-2867646-12.patch.txt545 byteswalangitan
#8 fix_coding_standards-2867646-8.patch5.87 KBadamzimmermann
#4 fix_coding_standards-2867646-4.patch6.09 KBadamzimmermann
#2 2867646-2.patch4.06 KBPavan B S

Comments

Pavan B S created an issue. See original summary.

Pavan B S’s picture

Pavan B S commented
Assigned: Pavan B S » Unassigned
StatusFileSize
new 2867646-2.patch4.06 KB

Applying the patch, please review

Pavan B S’s picture

Pavan B S commented
Status: Active » Needs review
adamzimmermann’s picture

adamzimmermann commented
Issue summary: View changes
StatusFileSize
new fix_coding_standards-2867646-4.patch6.09 KB
adamzimmermann’s picture

adamzimmermann commented
Issue summary: View changes

Most of the changes look good, but there were two spots were check_plain() was called on text that would have HTML in it. I reverted those back to calls to t(). If we want to change the placeholders to something more secure than ! to @ we could do that.

While I was at it, I made a couple other code standards fixes. Check it out and see if this addresses your original concerns.

Pavan B S’s picture

Pavan B S commented

@adamzimmermann thank you for the suggestions

markdorison’s picture

markdorison
he/him
commented
Status: Needs review » Needs work

@adamzimmermann Did you mean to include the binary file in the #4 patch? I think it is unrelated.

adamzimmermann’s picture

adamzimmermann commented
StatusFileSize
new fix_coding_standards-2867646-8.patch5.87 KB

Good catch. Remnants of old patches! Removed and new patch attached.

adamzimmermann’s picture

adamzimmermann commented
Status: Needs work » Needs review

markdorison’s picture

markdorison
he/him
commented
Status: Needs review » Fixed
walangitan’s picture

walangitan commented
Status: Fixed » Needs review
StatusFileSize
new edit_fix_coding_standards-2867646-12.patch.txt545 bytes

I have observed a

Fatal error: Access to undeclared static property: Recurly_Client::$apikey

as a result of apikey not being camel cased which appears to cause issues with the recurly php library recognizing the property. This patch should resolve this regression.

adamzimmermann’s picture

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

Looks good to me. Sometimes we can't follow all the coding standards it seems.

markdorison’s picture

markdorison
he/him
commented
Status: Reviewed & tested by the community » Fixed

This rings a bell for me; great catch.

markdorison’s picture

markdorison
he/him
commented

Committed the wrong patched; fixed.

Status: Fixed » Closed (fixed)

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