-
Notifications
You must be signed in to change notification settings - Fork 52
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
Rails 4 support #22
base: master
Are you sure you want to change the base?
Rails 4 support #22
Conversation
First off, thanks for contributing! 👍 to the Rails-related changes - no-one's actually told me they're using the hijack functionality, so I don't think there's any need to worry about backward compatibility, and it'd be good to have it working with Rails 4. I also agree that it's probably better not to hijack stdin/stdout - it's a big hack, and other tools that hook into test suites (e.g. simplecov) don't do it, so it's probably a bit surprising. Something seems to have gone wrong with the Travis build - was that you? I have one issue with the changes you've made to the output. I'm fine with waiting until processing has finished to output anything (especially when there's a test suite running), but comma-separated selectors are less trivial to manipulate with command-line Unix tools than line-separated. (Relatedly, if the test suite were running, I wouldn't expect it to pass, as it splits the output by line.) In addition, it's useful to be able to see what selectors are actually used in each CSS file for debugging purposes, though that needn't necessarily be on by default. Still, it's nice that your version takes up a lot less vertical space. Perhaps when running in hijack mode, we should log Deadweight's normal stdout (unused selectors) and stderr (stylesheets + used selectors therein) to two files, and just print "Found X selectors, Y used, Z unused" to the terminal. What do you think? |
Yeah I guess that's me. I didn't run the specs before pull-requesting as I wasn't sure whether the gem's still maintained and didn't want to put a lot of effort into it. But I will take a look at the specs soon, knowing now that you still care about deadweigth. 👍
A typical Rails app only has on
Sounds interesting. I will take a closer look at it. |
By the way, I'm working on a gem called Headhunter that automatically validates the HTML responses of Rack apps (using Tidy HTML). It was heavily inspired by Deadweight. I'm thinking about consolidating Deadweight's functionality into Headhunter to make it become a full-blown helper when trying to develop valid and clean HTML and CSS. |
Hm it seems that the specs run through without a problem:
Is there anything else that could have disturbed CI? |
Travis seems happy now, so that's good. |
I have changed the output to newlines again and override it for Rails. 👍
For this I don't have time at the moment, because (as mentioned before) I'm writing my own HTML&CSS-Validator at the moment. |
Just wanted to mention that the first version of my Headhunter gem is out! For Rails projects, I think it's a very powerful supplement (or even replacement) for your gem. From the docs:
Would be great to get some feedback from you. I'm currently working on adding some functionalities from your open pull requests into it which I also stumbled across meanwhile (as I copy&pasted a lot of your code 😉 : ). I hope you're not mad that I'm trying to make your Deadweight gem obsolete... |
PLEASE MAKE DEADWEIGHT OBSOLETE. I'll take a look when I've got the time. Sounds great. Can it be decoupled from Rails/Ruby? |
At the moment it's "married" to Rails. But it wouldn't be much of a problem to "divorce" it. As soon as the Rails version works well, I will think about adding a command line version (like you did with Deadweight). |
I never used this gem before, but I guess that it's not working anymore for newer versions of Rack and/or Rails, see especially my changes in
lib/deadweight/rack/capturing_middleware.rb
(respond_to :body
doesn't seem to be correct anymore) and inlib/deadweight/hijack/rails.rb
(application.css
is no longer understylesheets
, but inassets
, and it needs to be precompiled).I didn't find a more elegant way to load
application.css
intoDeadWeight
than to do a manualrake assets:precompile
from within Ruby. It would be nice to use the same source that Rails generates when ajs: true
test is run, but this is done only the first time such a request test is run, so we don't have it at this early stage. Maybe there is a way to trigger the generation manually somehow?I also refactored the console output: I don't think it's useful to hijack the log and send it to a file. I want to see what's happening with my tests, e.g. how many % already are done, etc.
It would be nice to prevent the output of
rake assets:precompile
from being displayed, but I didn't find a way to do it.As I only use this as a Rails gem, I didn't verify that all other functionality's still working.