search

xwp / wp-dependency-minification

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

Re-orginze the plugin as instantiated classes #36

Closed nash-ye closed 10 years ago

nash-ye commented 11 years ago

As discussed in the issue #31 , We want to re-organize the plugin to have more professional OOP style.

@westonruter said:

In general, I believe the smaller the class the better. The more that classes (and methods) do just one thing and do it well, the easier it will be to maintain the plugin and (especially) be able to write unit tests, as each component class of the plugin can be replaced with another component that implements the same interface. You know, mocks and dependency injection.

You can read the full-discussion in the issue #31

so.. as starting point we want to agree on the scheme pattern and the needed classes.

What is the classes names prefix shall we use? DepMin_ or Dependency_Minification_ ? How many classes can you separate the plugin to, and what they are? (write a names) ...etc

westonruter commented 11 years ago

Just off the top of my head, I think a class for Options and a class for Admin page would make sense (depends on #27). It would also be great if each minifier were implemented as a strategy, so that they could be easily swapped out (this relates to #7). So off the top of my head, we could have:

nash-ye commented 10 years ago

@westonruter Nice suggestions, and make full-sense for a starting point... I will start coding today.

nash-ye commented 10 years ago

@westonruter Which you prefer for the classes names prefix? DepMin or Dependency_Minification ? I think the shorter one is better.

westonruter commented 10 years ago

@nash-ye humm, I guess for the main plugin class it should remain Dependency_Minfication, but then the other classes in the plugin can freely use the shorter DepMin prefix. I can't wait until we can use PHP namespaces without worrying about compatibility!

westonruter commented 10 years ago

Pull requests #27 and #37 should get merged before the major refactor work.

nash-ye commented 10 years ago

Finally I had finished my exams today, I will complete this PR very soon, Thank you for your patience, I really appreciate it :)

Eid Mubarak!

westonruter commented 10 years ago

@nash-ye :+1: Happy Eid!

I still need to finish reviewing @shadyvb's work on #27 before a refactor should happen. I did leave some feedback for him.

nash-ye commented 10 years ago

Thanks @westonruter It was a nice happy day :) I give some notes to @shadyvb too.. I can't wait to finish this PR :100:

shadyvb commented 10 years ago

All set, i took care of all comments.

nash-ye commented 10 years ago

@shadyvb This has been open too long, Please tell me when you finish your PR.

westonruter commented 10 years ago

The PR has been merged into develop. Now we're just looking at applying PHP_CodeSniffer fixes.

nash-ye commented 10 years ago

Thanks @westonruter , So can we do this PR in 1.0 realase?

westonruter commented 10 years ago

Yes, sounds good.

nash-ye commented 10 years ago

This pull on prograss. The classes that I have done for now are DepMin_Options DepMin_Cache DepMin_Cache_Interface DepMin_Cache_In_WP_Options DepMin_Settings_Page

The first commit will be ready in the next two days... stay tuned!

westonruter commented 10 years ago

@nash-ye what is DepMin_Cache_In_WP_Options for?

nash-ye commented 10 years ago

@westonruter I have split the cache process to some small classes (slimier to WP_Object_Cache) so we can implement other ways to cache the compressed scripts/styles easily.

For example, we will could implement other caching methods like, separate SQLlite DB, Files...etc You will understand it more when I will push the first commit.

nash-ye commented 10 years ago

The first stage completed, please tell me your opinion.

What I have done? 1 Re-build the plugin as instantiated classes: Dependency_Minification DepMin_Admin DepMin_Options DepMin_SrcInfo DepMin_Handler DepMin_Collation DepMin_Minify DepMin_Minifier DepMin_Minifier_Default DepMin_Cache DepMin_Cache_Default DepMin_Cache_Interface

2- Fix some performance bugs and re-write some codes to look better :)

westonruter commented 10 years ago

@nash-ye could you please close this and open PR another to the develop branch?

westonruter commented 10 years ago

Since it is opened to the master branch, the PR includes all pending commits from develop not just the new ones you have done.

westonruter commented 10 years ago

@nash-ye Also, you can see that the Travis build failed due to warnings and errors reported by PHP_CodeSniffer. See https://travis-ci.org/x-team/wp-dependency-minification/jobs/13916005#L133

nash-ye commented 10 years ago

@westonruter Ok :+1: