This patch makes a number of comment and code style improvements based on recommendations from the coder module.

CommentFileSizeAuthor
#10 interdiff.txt46.08 KBmarkpavlitski
#10 memcache-coding-standards-1974812-10.patch83.97 KBmarkpavlitski
#5 memcache-coding-standards-1974812-5.patch41.79 KBmarkpavlitski
#1 memcache-coding-standards-1974812-1.patch39.11 KBmarkpavlitski

Comments

markpavlitski’s picture

markpavlitski commented
StatusFileSize
new memcache-coding-standards-1974812-1.patch39.11 KB
jeremy’s picture

jeremy commented
Status: Needs review » Postponed
Issue tags: +7.x-1.1

Sorry; I'm going to hold off on this for now -- once applied, it will invalidate most patches that have been waiting in the queue. I'd rather work through the queue first, and then look at doing a Coder cleanup.

Adding 7.x-1.1 tag to remind myself to revisit before we roll the next release.

markpavlitski’s picture

markpavlitski commented

That's fine, I've got it committed as a branch locally so will rebase after the next release.

jeremy’s picture

jeremy commented
Issue summary: View changes
Status: Postponed » Needs work

Feel free to regenerate now.

markpavlitski’s picture

markpavlitski commented
StatusFileSize
new memcache-coding-standards-1974812-5.patch41.79 KB

New patch attached against latest head.

markpavlitski’s picture

markpavlitski commented
Status: Needs work » Needs review
jeremy’s picture

jeremy commented

Just took a quick glance -- the very first diff looks to be making an incorrect change? Why is memcache.inc being moved up like that, in the dmemcache.inc file? Apparently this includes some manual changes too, for example "stromg" at the end of the diff instead of "string"...? Overall, I'll be happy to get this cleanup merged soon -- thanks!

markpavlitski’s picture

markpavlitski commented

I applied the original patch, cleaned it up and made some further fixes, but it looks like a couple of oddities got in.

I'll check again and reroll it in the morning.

markpavlitski’s picture

markpavlitski commented
Assigned: Unassigned » markpavlitski
Status: Needs review » Needs work
markpavlitski’s picture

markpavlitski commented
Status: Needs work » Needs review
StatusFileSize
new memcache-coding-standards-1974812-10.patch83.97 KB
new interdiff.txt46.08 KB

Patch and interdiff attached.

I've fixed all the odd merge issues I could find, and added some further clean-ups especially in the test files.

It's almost all cosmetic, but there are a couple of things to note:

  • hook_enable() moved from memcache.module to memcache.install.
  • MemCacheDrupal::wildcard_valid() (and all references) renamed to MemCacheDrupal::wildcardValid()

Given the possibility of commit conflicts with any other patches and assuming this patch doesn't introduce any significant issues, is it worth getting this committed and then creating a follow-up issue to track any remaining minor code style issues?

jeremy’s picture

jeremy commented
Status: Needs review » Fixed

Passed my review and testing -- thanks for sticking with this!
http://drupalcode.org/project/memcache.git/commitdiff/7c98e7f

Status: Fixed » Closed (fixed)

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