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

Fix: add missing kaminari-grape dependency and locked roar. #43

Merged
merged 1 commit into from
Feb 9, 2017

Conversation

dblock
Copy link
Collaborator

@dblock dblock commented Feb 9, 2017

No description provided.

@dangerpr-bot
Copy link

1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Generated by 🚫 danger

@dblock dblock merged commit 0192a79 into slack-ruby:master Feb 9, 2017
@dblock dblock deleted the fix-dependencies branch February 9, 2017 19:54
@swalberg
Copy link
Contributor

swalberg commented Feb 9, 2017

Hey, on this note I found I need gem 'representable', '~> 2.3.0' in my own Gemfile. Does the roar dependency address that or is that something else?

@dblock
Copy link
Collaborator Author

dblock commented Feb 9, 2017

Hm, it worked for my bots, but give it a try: remove that dependency in yours and update the dependency on slack-ruby-bot-server that I just released?

@swalberg
Copy link
Contributor

swalberg commented Feb 9, 2017

Ah, from my Gemfile.lock

 roar (1.0.4)
      representable (>= 2.0.1, < 2.4.0)

I found that if I upgrade representable to 3.x then I get a lot of test failures. I see that the latest roar has a "~> 3.0.0" dependency on representable.

Not sure if we're running into the same issue... IIRC the opts hash yielded to the link block went from having something to having nothing.

@dblock
Copy link
Collaborator Author

dblock commented Feb 9, 2017

Yes, I see the same thing. Didn't debug.

@dblock
Copy link
Collaborator Author

dblock commented Feb 9, 2017

It's on my todo list, but I won't get to it fast btw @swalberg. It would be great if someone (maybe you?) actually dug up what changed in representable to cause this.

@swalberg
Copy link
Contributor

swalberg commented Feb 9, 2017

I'll see what I can do!

@swalberg
Copy link
Contributor

Spent a few minutes with git bisect

trailblazer/roar@3540c8e broke it in the roar gem, which is where it updates to representable 2.4.0. Will dig into representable next.

@dblock
Copy link
Collaborator Author

dblock commented Feb 17, 2017

The representable problem was fixed in ruby-grape/grape-roar#21, maybe @swalberg wants to give it a shot at upgrading here?

@swalberg
Copy link
Contributor

Thanks, I'll give it a shot this weekend.

@swalberg
Copy link
Contributor

Yup, it works!

grape-roar (0.4.0)
representable (3.0.3)

swalberg added a commit to swalberg/slack-ruby-bot-server that referenced this pull request Feb 21, 2017
Fixes compatibility problems with representable 2.4+. An API change
meant that the current way of passing in the rack environment was
getting clobbered, and we were unable to access the context of the
request.

See
https://github.com/ruby-grape/grape-roar/pull/21/files
for the fix

and conversation in
slack-ruby#43
swalberg added a commit to swalberg/slack-ruby-bot-server that referenced this pull request Feb 21, 2017
Fixes compatibility problems with representable 2.4+. An API change
meant that the current way of passing in the rack environment was
getting clobbered, and we were unable to access the context of the
request.

See
https://github.com/ruby-grape/grape-roar/pull/21/files
for the fix

and conversation in
slack-ruby#43
swalberg added a commit to swalberg/slack-ruby-bot-server that referenced this pull request Feb 21, 2017
Fixes compatibility problems with representable 2.4+. An API change
meant that the current way of passing in the rack environment was
getting clobbered, and we were unable to access the context of the
request.

See
https://github.com/ruby-grape/grape-roar/pull/21/files
for the fix

and conversation in
slack-ruby#43

Change properties to `public` methods

See slack-ruby#45 - slack-ruby#45

trailblazer/representable@74155a9
changed to use `public_send` for these types of methods.

Use danger format
swalberg added a commit to swalberg/slack-ruby-bot-server that referenced this pull request Feb 21, 2017
Fixes compatibility problems with representable 2.4+. An API change
meant that the current way of passing in the rack environment was
getting clobbered, and we were unable to access the context of the
request.

See
https://github.com/ruby-grape/grape-roar/pull/21/files
for the fix

and conversation in
slack-ruby#43

Change properties to `public` methods

See slack-ruby#45 - slack-ruby#45

trailblazer/representable@74155a9
changed to use `public_send` for these types of methods.

Use danger format
swalberg added a commit to swalberg/slack-ruby-bot-server that referenced this pull request Feb 21, 2017
Fixes compatibility problems with representable 2.4+. An API change
meant that the current way of passing in the rack environment was
getting clobbered, and we were unable to access the context of the
request.

See
https://github.com/ruby-grape/grape-roar/pull/21/files
for the fix

and conversation in
slack-ruby#43

Change properties to `public` methods

See slack-ruby#45 - slack-ruby#45

trailblazer/representable@74155a9
changed to use `public_send` for these types of methods.

Use danger format
swalberg added a commit to swalberg/slack-ruby-bot-server that referenced this pull request Feb 21, 2017
Fixes compatibility problems with representable 2.4+. An API change
meant that the current way of passing in the rack environment was
getting clobbered, and we were unable to access the context of the
request.

See
https://github.com/ruby-grape/grape-roar/pull/21/files
for the fix

and conversation in
slack-ruby#43

Change properties to `public` methods

See slack-ruby#45 - slack-ruby#45

trailblazer/representable@74155a9
changed to use `public_send` for these types of methods.

Use danger format
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.

3 participants