Wordpress Coding Standards & Unit Testing Setup via Travis CI

[joshrathke]

I was using this for a project that needed to follow standards, so I figured I would go ahead and see if you wanted it integrated into the master branch on the main repo. The unit test I have been using to check everything is based on the Underscores theme by Automattic, which can be found here: https://github.com/Automattic/_s

Essentially it checks for WordPress Coding Standards, best practices and syntax errors. It fits nicely into a boilerplate theme because if you want to use it, it's there, if not, it has no effect on your project. But for me it's always a pain to set up unit testing. This may get a couple people off to a good start and encourage the spread of WP Coding Standards :)

At this point, with a few adjustments, I got the FoundationPress build to pass. Honestly didn't take too much. Major kudos to the whole community for contributing solid code. The WP Coding Standards can be fairly strict.

[olefredrik]

Hello. Thank you for this! Right now I'm on vacation. Will definitely see properly through this when I'm back. It's brilliant that you share your expertise. Will do code review and take your PR soon as I return from travel.

Cheers,
Ole Fredrik

[Aetles]

Great work!

[joshrathke]

Thanks guy's, at this point it would definitely be comforting to have a couple more eyes go over the code that was changed. The biggest things are function names that had to be tweaked from 'FoundationPress__' to 'foundationpress__' to eliminate camel casing and conform to WP Coding Standards, and making sure that my search and replace found all of the instances of those functions.

Also, I am definitely not a complete newbie, but also not an expert at setting up the .travis.yml file. But the one from the underscores repo is so well commented, I think everything in there is correct.

I checked everything as best as I could and didn't find any remaining bugs or incorrect function names, but if you find any please feel free to make a PR into my repo of FoundationPress. I'll merge it, and as long as this one hasn't been closed it should get pulled into this PR.

Seriously love this project! I think it has gotten a lot of people off the ground with a responsive framework for WordPress in no time!

[Aetles]

I haven't tested the code myself and I'm not familiar with Travis so I can't comment on that part. But I took a quick look at the diffs and it looks nice so far when it comes to adjusting to WordPress coding standards. I've been thinking more than once that I should do that, I'm glad you did!

I found one issue, the function FoundationPress_entry_meta() is renamed to foundationpress_entry_meta() in this pull request, but both content.php and single.php still contains a call to the function by the old name:

<?php FoundationPress_entry_meta(); ?>

And when we are talking about coding standards, I'd like to discuss some parts that could be changed as well.

For example the action hooks looks like this, with camelCase:
`do_action( 'foundationPress_post_before_comments' );``

The WordPress Naming Conventions say:

Use lowercase letters in variable, action, and function names (never camelCase).

…so I think the action hooks should be changed as well, if we want to go all-in on coding standard, right? That would break backward compatibility, though.

Class naming is also not following standard, it looks like this right now:
class FoundationPress_comments extends Walker_Comment{

The Naming Conventions say:

Class names should use capitalized words separated by underscores.

[joshrathke]

Awesome input! I definitely agree in terms of going all in. I have found that the Coding Standards "sniffs" throw flags at certain things sometimes and don't others. I thought it was just a bug on my local build, but it there must be something else going on.

Thanks for letting me know of those functions also. I was definitely a little concerned about stuff like that.

In terms of making sure that it's backwards compatible, do you think there are very many people coming back to download partial bits of code? I really don't know, I guess I have done it once or twice. What would be a good solution for that?

UPDATE: Went ahead and fixed those function names. And double checked all of the functions in /library/. Coudn't find any more errors in terms of function names. Class names, and Action Hooks still need to be discussed a little bit.

[Aetles]

Great!

I really have no idea if backward compatibility is a big issue. Personally it's not. FoundationPress, AFAIK, is meant to be a starter theme that you modify rather than update but I've seen a lot of issues here where people want to use it as a traditional theme (like all those issues about child theme support) so I guess some do update. On the other hand, FoundationPress has already had big changes many times over. I would guess that these action hooks are not frequently used. Anyway, that's for Ole to decide.

[olefredrik]

Hi. I do not see backward compatibility as a major issue in this case. The changes proposed are good and will make the theme a lot better with regard to follow wordpress standard conventions for coding. I have looked through diffs (from mobile safari, since I'm on vacation) and it looks very good! @josh-rathke, could you update your pull request with the input from @Aetles ? If both of you have gone through the code and see that everything is running fine, I'm happy to merge the PR from my phone. Well done, guys!

[joshrathke]

Hey @olefredrik, just to make sure, you want all of the Class Names and Action Hooks to be changed as well? I think those are the last remaining instances of code that need to be brought up to par with WordPress standards.

I am travelling the next few days, so not sure when I'll be able to get it done, @Aetles if you can get to it first, I'll merge the PR in with my Repo so that it gets pulled into this PR. Otherwise, I'll get to it when I can.

[olefredrik]

Hi there. Just came back from vacation. I've reviewed your code and tested that everything runs fine, so I merged your pull request. The class names and action hooks should be changed as well, but we can make a separate PR for that later. I'm a little busy the next two weeks, going to New York in a few days and have loads which must be arranged prior to it. Again, thank you for your contribution so far :)