xwp / wp-dependency-minification

Dependency Minification plugin for WordPress
http://wordpress.org/plugins/dependency-minification/
52 stars 10 forks source link

+ Adds a new Tabbed Settings page #27

Closed shadyvb closed 11 years ago

shadyvb commented 11 years ago
lkraav commented 11 years ago

Trying this functionality out, because s2member plugin doing some funky stuff with its enqueueing and needs to be excluded.

First comment: commit messages should be better.

http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

lkraav commented 11 years ago

Second, I think you have some whitespace issues in a7f751e9f19e6415e3f161cfb68b52055ea625d9 visible in at least the constants section in the very start of the commit. You should not be changing constant order or upstream code whitespace format.

westonruter commented 11 years ago

Re: commit messages, yes they should ideally be composed according to that note. Although the prefixing of each line with + or * is kinda helpful, this information can also be obtained with a log stat; so we try to have the commit summary be in the imperative form, like:

Add a new Tabbed Settings page

or

Expose default option for exclusion

Not a huge deal though.

lkraav commented 11 years ago

Functionality: what I'd like to see for URL exclusion is also a filter implementation, so code-based overrides become possible. Then admin UI should also use the filter to do its thing.

Starting with just the filter would probably be the minimal viable product that has a chance of getting merged quick.

lkraav commented 11 years ago

Meh, I guess the filter is already done https://github.com/x-team/wp-dependency-minification/blob/master/dependency-minification.php#L707

westonruter commented 11 years ago

Yes, re: filter, see the forums: http://wordpress.org/support/topic/woocommerce-not-supported?replies=2#post-4555036

shadyvb commented 11 years ago

@lkraav

shadyvb commented 11 years ago

Did some fixes to comply with WPCS in code, not only related to PR, only remaining issues are mostly related to indentation.

UPDATE: @westonruter I just figured out that i shouldn't have fixed CS issues on this PR, it'd make it quite hard for code review. Sorry!

westonruter commented 11 years ago

This has been open too long, sorry @shadyvb. Now that it is in develop we can work toward the next release. One thing I need to do is add Travis CI.

westonruter commented 11 years ago

Opened PR https://github.com/x-team/wp-dependency-minification/pull/44 to track additional fixes for PHP_CodeSniffer, and for the 1.0 release.