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

Remove dependency on ActiveSupport #27

Merged
merged 1 commit into from
Jan 28, 2020
Merged

Conversation

BrianBorge
Copy link
Contributor

@BrianBorge BrianBorge commented Jan 28, 2020

Removes Consult's dependency on ActiveSupport. It's one less dependency to manage and a smaller deployment package for projects that depend on Consult that aren't Rails apps.

Addresses #26

@BrianBorge BrianBorge self-assigned this Jan 28, 2020
- One less dependency to manage
- Smaller deployment package for projects that depend on Consult
- Replaces String#present? by converting values to string and then checking that length is greater than 0 (nil.to_s is an empty string)
- Replaces Hash#deep_symbolize_keys! and Hash#deep_merge with their ActiveSupport implementations
@BrianBorge BrianBorge force-pushed the remove-dep-on-activesupport branch from ed555e6 to c4430c2 Compare January 28, 2020 00:51
Copy link
Contributor

@aharpervc aharpervc left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@sixfeetover sixfeetover left a comment

Choose a reason for hiding this comment

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

LGTM as well

@BrianBorge
Copy link
Contributor Author

BrianBorge commented Jan 28, 2020

Should a revision bump and changelog go along with this? 0.9.0 to 0.9.1? I'm not sure - looking for guidance here.

@aharpervc
Copy link
Contributor

Should a revision bump and changelog go along with this? 0.9.0 to 0.9.1? I'm not sure - looking for guidance here.

That seems reasonable. 0.9.1 or 0.10.0 is fine with me, either way

@sixfeetover
Copy link
Member

Yes, but do it post-merge. You can also be added to the gem committers if you aren't already.

@BrianBorge BrianBorge merged commit 2112be4 into master Jan 28, 2020
@BrianBorge BrianBorge deleted the remove-dep-on-activesupport branch January 28, 2020 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants