Skip to content

Makes keyword arguments usage compatible w/ Ruby 3 #123

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

Merged
merged 1 commit into from
May 7, 2021

Conversation

walski
Copy link
Contributor

@walski walski commented Jan 4, 2021

In Ruby 3 keyword arguments are completely decoupled from positional arguments (see e.g. ruby-lang.org News).

This fix makes the LinkFormatter compatible again with the new semantics of keyword arguments in Ruby 3.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
In Ruby 3 keyword arguments are completely *decoupled* from positional arguments (see e.g. [ruby-lang.org News](https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/)).

This fix makes the LinkFormatter compatible again with the new semantics of keyword arguments in Ruby 3.
@matthutchinson
Copy link

matthutchinson commented Feb 18, 2021

Looks good, and will work when calling Slack::Notifier::Util::LinkFormatter.format("some message"), but the initializer should probably be fixed too (since it's a public method).

Some people may be calling it like this;

Slack::Notifier::Util::LinkFormatter.new("some message").formatted

@6temes
Copy link

6temes commented Apr 15, 2021

Is this gem still maintained? The last merge was in 2008.

@stevenosloan stevenosloan changed the base branch from master to main May 7, 2021 17:40
@stevenosloan
Copy link
Member

Just noting I'm merging this over #119 for the more descriptive commit message. Same change so I had to pick somehow 😅.

After adding some smoke tests around the usage of the initializer & formatted methods directly they don't appear to be affected by the keyword arg changes.

@stevenosloan stevenosloan merged commit b3ff6ba into slack-notifier:main May 7, 2021
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.

None yet

4 participants