While I was making some changes on another issue, I ran coder module and took care of all the warnings that it found.

(this patch includes the changes in Issue #267500 #267500: activitystream_feed gives no warning if simplepie.inc is not available )

CommentFileSizeAuthor
#9 activitystream-code_cleanup-267534-5.patch24.7 KBeojthebrave
#2 activitystream-codecleanup-267534.patch56.02 KBeojthebrave
activitystream_cleanup.patch408.97 KBdesignerbrent

Comments

HorsePunchKid’s picture

HorsePunchKid commented

I'd be happy to do the same for the 5.x branch if it's likely to get committed.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
StatusFileSize
new activitystream-codecleanup-267534.patch56.02 KB

designerbrent, Great work! Download and tested your patch. Applied cleanly to 6.x-2.x-dev. Then ran coder.module and there were still a couple of warnings. All of them had to do with capitalization of NULL, FALSE, and TRUE. I fixed the few errors and re-rolled the patch.

It would be great if this could get committed. I can re-roll all of my E_Notice related patches against CVS once you've committed this patch if needed.

akalsey’s picture

akalsey commented

My local copy of 5.x already had the tabs to spaces cleanup. I'll check in those changes shortly, and if you want to do a code cleanup on the dev version of 5.x I'd be happy to commit it.

akalsey’s picture

akalsey commented
Status: Needs review » Needs work

Looks like the patch doesn't apply, most likely because it contains changes that are also in #267500: activitystream_feed gives no warning if simplepie.inc is not available, which had already been applied and committed.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented

I'll try and re-roll against the current version in the next couple of days.

designerbrent’s picture

designerbrent commented
Assigned: designerbrent » Unassigned

Sorry.. I haven't had the time to work on this more lately.

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
Status: Needs work » Needs review

Okay, this patch cleans up everything pointed out by coder.module, along with a few more cosmetic tweaks. Mostly adding commas after the last element of an array, and deleting a few un-necessary newlines here and there.

You need to apply this patch first in order for this to apply cleanly. #287566: PHP Notices on user/*/edit and user/*/edit/Activity Stream

akalsey’s picture

akalsey commented
Status: Needs review » Needs work

No patch attached...

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
StatusFileSize
new activitystream-code_cleanup-267534-5.patch24.7 KB

Oops. Here ya go.

akalsey’s picture

akalsey commented
Status: Needs work » Fixed

Commited in 6.x-1x (cvs branch DRUPAL-6--1). I just noticed that the branch name was off (was declaring this to be 2.x instead of 1.x of the module), so I branched appropriately.

spiffyd’s picture

spiffyd commented
Priority: Normal » Minor
Status: Fixed » Active

Did you forget to add class="permalink" to the "#" permalinks in the feed module? It seems that all other modules have the class except feeds.

akalsey’s picture

akalsey commented

Hmm. The 5.x module has the class. I'm guessing that one of the patches applied to 6.x regressed this at some point.

akalsey’s picture

akalsey commented
Status: Active » Closed (fixed)

The 6.x version does have this class set. See http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/activitystr...

spiffyd’s picture

spiffyd commented

@akalsey, I believe I found the root of the issue, check out my findings in this other thread: http://drupal.org/node/288018#comment-964170