Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for Packer #173

Merged
merged 7 commits into from
Apr 19, 2012
Merged

Added support for Packer #173

merged 7 commits into from
Apr 19, 2012

Conversation

maximilian-walter
Copy link
Contributor

I added the Filter PackerFilter which uses the PHP-Port of Dean Edwards's Packer to compress JavaScript.

*/
class PackerFilter implements FilterInterface
{
protected $encoding = 'None';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use spaces for the indentation, not tabs

@maximilian-walter
Copy link
Contributor Author

Done! Thanks for the feedback.

@kriswallsmith
Copy link
Owner

Looks good to me. Could you please add a test for this filter? This is a good example:
https://github.com/kriswallsmith/assetic/blob/master/tests/Assetic/Test/Filter/CssMinFilterTest.php

@maximilian-walter
Copy link
Contributor Author

Is still something missing? :)

@kriswallsmith
Copy link
Owner

Sorry, I wasn't notified when you added this commit. I will review soon. Thanks!

@stof
Copy link
Collaborator

stof commented Feb 15, 2012

github never notifies when new commits are pushed in a PR.

@kriswallsmith
Copy link
Owner

oui

@everzet
Copy link
Collaborator

everzet commented Feb 15, 2012

Oh, this american accent :-D

@maximilian-walter
Copy link
Contributor Author

Good to know. I didn't know that, sorry.

@kriswallsmith
Copy link
Owner

Yeah, it's a good idea to post a comment when you push to a PR to notify the folks subscribed.

@stof
Copy link
Collaborator

stof commented Feb 28, 2012

@kriswallsmith is there anything missing according to you ?

@kriswallsmith
Copy link
Owner

Please add the $_SERVER var to phpunit.xml.dist and we're all set.

@stof
Copy link
Collaborator

stof commented Feb 28, 2012

And please add it in the Travis setup (you will need to rebase your branch to have it) so that Travis clones the needed repo and can run the test

@maximilian-walter
Copy link
Contributor Author

Okay, seems that I need some help here... I have done a rebase on my local repository:

git fetch upstream
git rebase upstream/master

But now I can't push it to GitHub...

 ! [rejected]        master -> master (non-fast forward)
 error: failed to push some refs to 'git@github.com:maximilian-walter/assetic.git'

What should I do now?

@stof
Copy link
Collaborator

stof commented Feb 29, 2012

you need to force the push of the branch. A simple push will fail and this is logical as you rewrote the history (git tries to avoid you doing mistakes if you did not want to rewrite it)

git push origin master --force

@stof
Copy link
Collaborator

stof commented Apr 3, 2012

@maximilian-walter ping. Could you rebase your branch ?

Btw, it would be great to add Packer to the vendors downloaded during the travis setup (and adding the path in the travis phpunit config) so that the test does not need to be skipped.

@stof
Copy link
Collaborator

stof commented Apr 15, 2012

@maximilian-walter ping

@maximilian-walter
Copy link
Contributor Author

Sorry, I was very busy the last time. I could rebase my branch and added Packer to the Travis setup, thanks for your help. Can you check it, please.

@@ -11,5 +11,6 @@ before_script:
- git clone https://github.com/leafo/lessphp.git vendor/lessphp --quiet --depth 1
- git clone https://github.com/mrclay/minify.git vendor/minify --quiet --depth 1
- svn checkout http://cssmin.googlecode.com/svn/trunk/ vendor/cssmin --quiet
- wget --quiet -O javascript-packer.zip "http://joliclic.free.fr/php/javascript-packer/telechargement.php?id=2&action=telecharger" && mkdir -p vendor/packer && unzip -qq javascript-packer.zip -d vendor/packer; rm javascript-packer.zip
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong indentation (which will break things as it makes the YAML invalid)

@maximilian-walter
Copy link
Contributor Author

The wrong indentation is fixed!

stof added a commit that referenced this pull request Apr 19, 2012
@stof stof merged commit 41ab8ea into kriswallsmith:master Apr 19, 2012
@stof
Copy link
Collaborator

stof commented Apr 19, 2012

Thanks

@FredoVelcro-zz
Copy link

Can someone help me to configure this filter in Symfony2 ?
Thanks a lot ! ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants