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

WIP caching module #39

Closed
wants to merge 7 commits into from
Closed

WIP caching module #39

wants to merge 7 commits into from

Conversation

ajb
Copy link

@ajb ajb commented Dec 3, 2015

@ageweke: I could use your help here if you have a second. I'm using the following command to run the tests:

bundle exec rspec spec/rails/cacheable_method_system_spec.rb

And it seems that there's some sort of error... but instead of logging the error to the console, it appears to get swallowed by the oop_rails_server.

Any ideas? Or is there a better development workflow?

@ajb
Copy link
Author

ajb commented Dec 3, 2015

(I should note that the caching implementation in here isn't really ready for review yet. Just need some help with tooling.)

@ageweke
Copy link
Owner

ageweke commented Dec 3, 2015

Adam,

Unfortunately, I'm on my phone only at the moment, and won't be able to take a look at this directly until this evening. However, when I have a problem with a Rails spec that I can't otherwise figure out, I do this: run the spec, then cd into the temporary directory that is that spec's Rails root under fortitude/tmp/specs, then bundle exec rails server and hit the relevant controller action from there. This lets you see the Rails output and HTML output just as you would in a normal app, and is pretty easy. If you have trouble figuring out the exact directory, just rm -rf fortitude/tmp and re-run -- it'll be the only directory there.

I'll dig in more when I get home tonight, but hopefully that should give you a start in the mean time!

Andrew

On Dec 3, 2015, at 8:26 AM, Adam Becker [email protected] wrote:

@ageweke: I could use your help here if you have a second. I'm using the following command to run the tests:

bundle exec rspec spec/rails/cacheable_method_system_spec.rb

And it seems that there's some sort of error... but instead of logging the error to the console, it appears to get swallowed by the oop_rails_server.

Any ideas? Or is there a better development workflow?

You can view, comment on, or merge this pull request online at:

#39

Commit Summary

wip
File Changes

M lib/fortitude/widget.rb (2)
A lib/fortitude/widget/caching.rb (28)
A spec/rails/cacheable_method_system_spec.rb (10)
A spec/rails/templates/static_method_system_spec/app/controllers/cacheable_method_system_spec_controller.rb (11)
A spec/rails/templates/static_method_system_spec/app/views/cacheable_method_system_spec/localization.rb (7)
Patch Links:

https://github.com/ageweke/fortitude/pull/39.patch
https://github.com/ageweke/fortitude/pull/39.diff

Reply to this email directly or view it on GitHub.

@ajb
Copy link
Author

ajb commented Dec 3, 2015

Thanks, very helpful!

On Thu, Dec 3, 2015 at 1:06 PM, Andrew Geweke [email protected]
wrote:

Adam,

Unfortunately, I'm on my phone only at the moment, and won't be able to
take a look at this directly until this evening. However, when I have a
problem with a Rails spec that I can't otherwise figure out, I do this: run
the spec, then cd into the temporary directory that is that spec's Rails
root under fortitude/tmp/specs, then bundle exec rails server and hit the
relevant controller action from there. This lets you see the Rails output
and HTML output just as you would in a normal app, and is pretty easy. If
you have trouble figuring out the exact directory, just rm -rf
fortitude/tmp and re-run -- it'll be the only directory there.

I'll dig in more when I get home tonight, but hopefully that should give
you a start in the mean time!

Andrew

On Dec 3, 2015, at 8:26 AM, Adam Becker [email protected]
wrote:

@ageweke: I could use your help here if you have a second. I'm using the
following command to run the tests:

bundle exec rspec spec/rails/cacheable_method_system_spec.rb

And it seems that there's some sort of error... but instead of logging
the error to the console, it appears to get swallowed by the
oop_rails_server.

Any ideas? Or is there a better development workflow?

You can view, comment on, or merge this pull request online at:

#39

Commit Summary

wip
File Changes

M lib/fortitude/widget.rb (2)
A lib/fortitude/widget/caching.rb (28)
A spec/rails/cacheable_method_system_spec.rb (10)
A
spec/rails/templates/static_method_system_spec/app/controllers/cacheable_method_system_spec_controller.rb
(11)
A
spec/rails/templates/static_method_system_spec/app/views/cacheable_method_system_spec/localization.rb
(7)
Patch Links:

https://github.com/ageweke/fortitude/pull/39.patch
https://github.com/ageweke/fortitude/pull/39.diff

Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#39 (comment).

Adam Becker
(951) 9-BECKER
@AdamJacobBecker

@@ -25,6 +25,11 @@ def render(widget_class, template_handler, local_assigns, is_partial, &block)
end

widget = widget_class.new(needed_assigns)

if options[:pathname]
widget.instance_variable_set(:@virtual_path, options[:pathname])
Copy link
Author

Choose a reason for hiding this comment

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

In order for ActionView::Helpers::CacheHelper to work properly, the @virtual_path instance variable must be set: https://github.com/rails/rails/blob/215f86c3483cf300b2ccd6d5d8b97f9c9bf547f5/actionview/lib/action_view/helpers/cache_helper.rb#L230

See discussion in template_handler.rb below:

@ajb
Copy link
Author

ajb commented Dec 3, 2015

@ageweke: ready for your feedback!

@ajb ajb mentioned this pull request Dec 4, 2015
@ajb ajb closed this Sep 19, 2016
@ajb ajb mentioned this pull request Sep 19, 2016
@ajb
Copy link
Author

ajb commented Sep 19, 2016

See an updated PR in #54.

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.

2 participants