Remove activesupport dependency #325

ianks · 2020-05-28T14:52:24Z

For non-rails shops, depending on AS can be a non-starter. Luckily, this gem only relied on two small AS features from what I can tell: class_attribute, String#blank? and one occasion of ActiveSupport#HashWithIndifferentAccess. It was easy to adjust and as a result, it should be easier to adopt.

Thank you for this gem, our team is very excited to start using it!

dblock

I am open to this. See minor comments + there needs to be an UPGRADING entry as someone might be bringing in AS via this gem, and a major version bump.

dblock · 2020-05-28T17:57:01Z

lib/tasks/web.rake


 namespace :slack do
  namespace :web do
    namespace :api do
      desc 'Update Web API.'
      task update: [:git_update] do
+        require 'active_support'


Should be a developement dependency for this, otherwise the task will fail anyway without the gem installed separately. Also, maybe code can be fixed not to need it either?

Since I don't know what this task is supposed to do, I do not feel comfortable refactoring it. But I will make it a development dependency 😄

I think you should give it a shot. It updates the web API from its reference, and shouldn't need AS either. As long as you can run it and it succeeds and produces no diff, we're good.

dblock · 2020-05-28T17:58:07Z

lib/slack/messages/message.rb

@@ -2,10 +2,6 @@
 module Slack
  module Messages
    class Message < Hashie::Mash
-      def presence


This is an actual Slack field. Aren't we breaking an API here? Should have had tests for it though either way.

I checked this a few times, and all the specs are indeed passing, and behavior seems correct.

I think we're missing a spec. This overrides Object#presence, so when there's a presence field in the result you get its value and not the return of that object. Either way, why remove it? I think you should put this code back, and if you care write a spec or open an issue and I will get to it.

dblock · 2020-06-08T11:49:10Z

@ianks Want to finish this?

dblock

I am still uncomfortable with the presence changes and I think you should fix the update task so we can really get rid of AS.

dblock · 2020-06-10T13:45:49Z

lib/slack/messages/message.rb

@@ -2,10 +2,6 @@
 module Slack
  module Messages
    class Message < Hashie::Mash
-      def presence


I think we're missing a spec. This overrides Object#presence, so when there's a presence field in the result you get its value and not the return of that object. Either way, why remove it? I think you should put this code back, and if you care write a spec or open an issue and I will get to it.

dblock · 2020-06-10T13:47:30Z

lib/slack/web/pagination/cursor.rb

@@ -32,14 +32,14 @@ def each

                client.logger.debug("#{self.class}##{__method__}") { e.to_s }
                retry_count += 1
-                sleep(e.retry_after.seconds)
+                sleep(e.retry_after)


How did the type of retry_after change?

retry_after is an integer, so calling .seconds was effectively a no-op.

dblock · 2020-06-10T13:48:23Z

lib/tasks/web.rake


 namespace :slack do
  namespace :web do
    namespace :api do
      desc 'Update Web API.'
      task update: [:git_update] do
+        require 'active_support'


I think you should give it a shot. It updates the web API from its reference, and shouldn't need AS either. As long as you can run it and it succeeds and produces no diff, we're good.

dblock · 2020-06-10T13:49:22Z

UPGRADING.md

+
+See [#325](https://github.com/slack-ruby/slack-ruby-client/pull/325) for more information.
+
+```


Bad copy-paste?

ianks · 2020-06-16T16:46:57Z

I think we're missing a spec. This overrides Object#presence, so when there's a presence field in the result you get its value and not the return of that object. Either way, why remove it? I think you should put this code back, and if you care write a spec or open an issue and I will get to it.

@dblock All specs pass without the presence method, but when I attempt to define it myself, I cannot get the specs to pass regardless of whether I use :presence or 'presence'. Object#presence is an ActiveSupport method, FWIW

Depending on the context, things break. Could you could clone down to take a look?

dblock

@dblock All specs pass without the presence method, but when I attempt to define it myself, I cannot get the specs to pass regardless of whether I use :presence or 'presence'. Object#presence is an ActiveSupport method, FWIW

OK, I get it, this is why we put it here, to override the behavior that AS was bringing

dblock · 2020-06-16T17:41:36Z

UPGRADING.md

@@ -1,6 +1,19 @@
 Upgrading Slack-Ruby-Client
 ===========================

+### Upgrading to >= 0.16.0
+
+As of 0.16.0, we no longer depend on `activesupport` as a dependency. This means that if you were relying on `activesupport` functionality unintentionally, certain things in you code may break. To fix this, you have two options:


Let's say this simpler

As of 0.16.0 slack-ruby-client no longer requires activesupport. If you are using activesupport, add gem 'activesupport' to your Gemfile..

dblock · 2020-06-16T17:43:22Z

UPGRADING.md

+
+As of 0.16.0, we no longer depend on `activesupport` as a dependency. This means that if you were relying on `activesupport` functionality unintentionally, certain things in you code may break. To fix this, you have two options:
+
+1. Update your code to use pure Ruby, without ActiveSupport methods (*recommended*).


This is really none of our business in this library :)

dblock

I'm good with this. Can you please use my shorter suggested UPGRADING note and I'll merge. Feel free to squash too.

dblock · 2020-06-16T17:44:34Z

CHANGELOG.md

@@ -1,5 +1,7 @@
 ### 0.15.0 (Next)

+* [#325](https://github.com/slack-ruby/slack-ruby-client/pull/325): Remove activesupport dependency - [@ianks](https://github.com/ianks).
+* [#326](https://github.com/slack-ruby/slack-ruby-client/pull/326): Update new methods - [@wasabigeek](https://github.com/wasabigeek).


This shouldn't be in this PR. remove and rebase.

dblock · 2020-06-16T17:44:58Z

UPGRADING.md

@@ -1,6 +1,19 @@
 Upgrading Slack-Ruby-Client
 ===========================

+### Upgrading to >= 0.16.0


This should be 0.15.0 AFAIK, not 0.16.0.

dblock · 2020-06-28T14:10:03Z

Merged via 7494411

dblock requested changes May 28, 2020

View reviewed changes

ianks added 4 commits June 9, 2020 17:13

Remove activesupport dependency

c13e534

Add changelog entry

0530077

Require AS lazily in rake task to avoid accidental load

876dd64

Make activesupport a dev dependency

987ad2a

ianks force-pushed the remove-activesupport-dependency branch from 94ba04d to 987ad2a Compare June 9, 2020 21:15

ianks requested a review from dblock June 9, 2020 21:15

Add note to UPGRADING.md

ad9a296

dblock requested changes Jun 10, 2020

View reviewed changes

ianks added 3 commits June 16, 2020 12:50

Fix changelog entry

bbdfd82

Fix merge conflicts

bb7c85f

Attempt to remove AS from rake task

4f13a63

ianks requested a review from dblock June 16, 2020 16:54

dblock reviewed Jun 16, 2020

View reviewed changes

dblock approved these changes Jun 16, 2020

View reviewed changes

dblock reviewed Jun 16, 2020

View reviewed changes

dblock closed this Jun 28, 2020

This was referenced Aug 2, 2020

Added active-support dependency. slack-ruby/slack-ruby-bot#271

Merged

Pongbot does not run on linux mint 19 slack-ruby/slack-ruby-bot#270

Closed

jmanian mentioned this pull request Apr 12, 2021

Suggestion: Replace Hashie::Mash with OpenStruct #371

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove activesupport dependency #325

Remove activesupport dependency #325

ianks commented May 28, 2020

dblock left a comment

dblock May 28, 2020

ianks Jun 9, 2020

dblock Jun 10, 2020

dblock May 28, 2020

ianks Jun 9, 2020

dblock Jun 10, 2020

dblock commented Jun 8, 2020

dblock left a comment

dblock Jun 10, 2020

dblock Jun 10, 2020

ianks Jun 16, 2020

dblock Jun 10, 2020

dblock Jun 10, 2020

ianks commented Jun 16, 2020 •

edited

Loading

dblock left a comment

dblock Jun 16, 2020

dblock Jun 16, 2020

dblock left a comment

dblock Jun 16, 2020

dblock Jun 16, 2020

dblock commented Jun 28, 2020


		See [#325](https://github.com/slack-ruby/slack-ruby-client/pull/325) for more information.

		```


		As of 0.16.0, we no longer depend on `activesupport` as a dependency. This means that if you were relying on `activesupport` functionality unintentionally, certain things in you code may break. To fix this, you have two options:

		1. Update your code to use pure Ruby, without ActiveSupport methods (recommended).

Remove activesupport dependency #325

Remove activesupport dependency #325

Conversation

ianks commented May 28, 2020

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dblock commented Jun 8, 2020

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ianks commented Jun 16, 2020 • edited Loading

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dblock commented Jun 28, 2020

ianks commented Jun 16, 2020 •

edited

Loading