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

Use new dry-configurable api 'default:' #50

Merged
merged 1 commit into from
Nov 15, 2021

Conversation

ahmeij
Copy link
Contributor

@ahmeij ahmeij commented Oct 2, 2021

The dry-configurable gem has been updated with a new preferred API, it is now throwing deprecation warnings like this:

dry-configurable-0.13.0/lib/dry/configurable/dsl.rb:28:in `initialize' [dry-configurable] default value as positional argument to settings is deprecated and will be removed in the next major version
Provide a `default:` keyword argument instead

This pull-request implements the new syntax.

@lukassup
Copy link
Contributor

@thde ping?

@thde
Copy link
Member

thde commented Oct 26, 2021

Wouldn't this make it incompatible to users who are using an older version of dry-configurable? If so the netbox-client-ruby.gemspecwould need to be changed accordingly.

@lukassup
Copy link
Contributor

I think we can pin dependency dry-configurable~>0.13.0 and release a new gem version so users who upgrade would require the updated version

@thde
Copy link
Member

thde commented Nov 4, 2021

I'm happy to merge it, if you find the time to create a PR. Sadly I won't be able to do it myself.

@lukassup
Copy link
Contributor

lukassup commented Nov 4, 2021

Whats the release process? Merge this PR to master, then bump Gem version in a separate PR and will it build & release automatically with CI or ...?

@thde
Copy link
Member

thde commented Nov 5, 2021

I can do the release. Just create a MR with the current commits and the change to the netbox-client-ruby.gemspec :).

@lukassup
Copy link
Contributor

@thde are you going to merge this PR first? And then do the release?

@thde
Copy link
Member

thde commented Nov 11, 2021

I won't be able to merge this MR, as it doesn't include the changes to the .gemspec file. If you're willing to create a new one includeing the changes in this MR and to the .gemspec file, I'm happy to merge and release.

@thde thde merged commit 9467ef4 into ninech:master Nov 15, 2021
@ahmeij ahmeij deleted the hotfix-dry-configurable-api branch January 4, 2022 10:18
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