ruby-grape / grape-rabl

Use rabl with grape
MIT License
136 stars 29 forks source link

Enabled to use layout template in rabl #22

Closed koko1000ban closed 10 years ago

koko1000ban commented 10 years ago

hi

i made this change to use layout template like this

koko1000ban commented 10 years ago

I look at RABL's .travis.yml file and they don't test on JRuby. but, grape-rabl has JRuby support. Is there any reason? i think grape-rabl should remove JRuby support at travis file until rabl will support.

LTe commented 10 years ago

@koko1000ban thanks for pull request. You are awesome! I have a few comments.

LTe commented 10 years ago

I look at RABL's .travis.yml file and they don't test on JRuby. but, grape-rabl has JRuby support. Is there any reason?

No :) But I think .travis.yml should not be part of this pull request. Can you remove this commit?

@koko1000ban can you also update https://github.com/LTe/grape-rabl/blob/master/CHANGELOG.md ? You can based on this commit (https://github.com/LTe/grape-rabl/commit/56da0a5bcecb16501cdd93ac25f3b6ca6d7a86f0#diff-4ac32a78649ca5bdd8e0ba38b7006a1e). Checkout out Next Release section.

koko1000ban commented 10 years ago

@LTe Thank you for your comments. i removed that commit. but, test is failed due to same problem of this probably. What would be the best thing to do?

dblock commented 10 years ago

I think the Travis failure is legit, it says:

1) Grape::Rabl layout default proper render with default layout
     Failure/Error: get("/about")
     LocalJumpError:
       yield called out of block
     # ./spec/views/layout_test/layouts/application.rabl:4:in `(eval)'
     # ./spec/../lib/grape-rabl/formatter.rb:21:in `call'
     # ./spec/../lib/grape-rabl/formatter.rb:50:in `rabl'
     # ./spec/../lib/grape-rabl/formatter.rb:17:in `call'
     # ./spec/grape_rabl_layout_spec.rb:28:in `(root)'
  2) Grape::Rabl layout tilt layout is setup proper render with specified layout
     Failure/Error: get("/about")
     LocalJumpError:
       yield called out of block
     # ./spec/views/layout_test/layouts/another.rabl:2:in `(eval)'
     # ./spec/../lib/grape-rabl/formatter.rb:21:in `call'
     # ./spec/../lib/grape-rabl/formatter.rb:50:in `rabl'
     # ./spec/../lib/grape-rabl/formatter.rb:17:in `call'
     # ./spec/grape_rabl_layout_spec.rb:46:in `(root)'

There's a yield in the template. I am not familiar with the Rabl syntax/implementation but this probably means it's doing something wrong.

dblock commented 10 years ago

Bump. What do we want to do with this?

koko1000ban commented 10 years ago

@dblock :+1:

dblock commented 10 years ago

Dug a bit deeper without success. I agree that since Rabl itself doesn't support JRuby we should just remove it from here. I'll merge this PR.

dblock commented 10 years ago

https://github.com/nesquena/rabl/issues/544

dblock commented 10 years ago

Merged in https://github.com/LTe/grape-rabl/commit/1fbfbd58c3fb320be1b52b3247fda2a23cacc9fc.