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

Include css and js from plugins if they are there (without updated gems) #89

Merged
merged 2 commits into from
May 26, 2015

Conversation

mattgibson
Copy link

This is a cleaner version of #85, but without the updated gems.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.28%) to 83.64% when pulling 890716d on mattgibson:plugin-assets into 10f2d2e on resque:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.28%) to 83.64% when pulling 890716d on mattgibson:plugin-assets into 10f2d2e on resque:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.28%) to 83.64% when pulling 890716d on mattgibson:plugin-assets into 10f2d2e on resque:master.

@mattgibson
Copy link
Author

Hmm. Looks like it needs the gems updated for 2.2

@mcfiredrill
Copy link

@mattgibson Thank you for doing this. 💛

I see, perhaps you could make a PR to fix the Gems to pass on 2.2?
After that is merged, you (and other people with currently failing PRs) could rebase.

@mcfiredrill
Copy link

@mattgibson now that we merged your other PR for the Gems, could you rebase this PR?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.13%) to 84.19% when pulling 41f6163 on mattgibson:plugin-assets into c09a9b4 on resque:master.

@mcfiredrill
Copy link

Not sure what the coveralls failure is about really, but this looks good.
Anyone else out there wanna 👍 this ?

@kirillplatonov
Copy link
Collaborator

@mattgibson Hi. Great job! 👏

Thinking about loading plugin's assets - not all plugins has its own assets. With stylesheet_link_tag and javascript_include_tag it will generate an error. Here's example with resque-scheduler plugin:
Example

I think we need to think about this problem together. Could you give the link to the plugin, for which try to load assets, please?

@mattgibson
Copy link
Author

@kirillplatonov Yes, this occurred to me too. I will see if I can add a file_exists check of some sort.

@kirillplatonov
Copy link
Collaborator

@mattgibson Probably Rails.application.assets.find_asset("application.css") will help?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.42%) to 83.9% when pulling 984c059 on mattgibson:plugin-assets into c09a9b4 on resque:master.

@kirillplatonov
Copy link
Collaborator

Looks fine now 👍

@mattgibson
Copy link
Author

Thanks. There's no tests for this, but I'm not sure of the best way of testing, or even if it's desirable. The plugins are in separate gems, so would it need a dummy gem of some sort?

@mcfiredrill
Copy link

@mattgibson @kirillplatonov
Thanks guys, seems good to merge now.

In the future, perhaps a test plugin (it doesn't have to actually do anything) could be added to the existing tests/test dummy app.

mcfiredrill added a commit that referenced this pull request May 26, 2015
Include css and js from plugins if they are there (without updated gems)
@mcfiredrill mcfiredrill merged commit 389f448 into resque:master May 26, 2015
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.

4 participants