Ruby Warnings #270
Ruby Warnings #270
Conversation
| LONG = 'AcinusFallumTrompetumNullunCreditumVisumEstAtCuadLongumEtCefallum.com' | ||
| it "should convert '#{LONG}' correctly" do | ||
| expect(Addressable::IDNA.to_unicode(LONG)).to eq(LONG) | ||
| long = 'AcinusFallumTrompetumNullunCreditumVisumEstAtCuadLongumEtCefallum.com' |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| LONG = 'AcinusFallumTrompetumNullunCreditumVisumEstAtCuadLongumEtCefallum.com' | ||
| it "should convert '#{LONG}' correctly" do | ||
| expect(Addressable::IDNA.to_ascii(LONG)).to eq(LONG) | ||
| long = 'AcinusFallumTrompetumNullunCreditumVisumEstAtCuadLongumEtCefallum.com' |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| LONG = 'AcinusFallumTrompetumNullunCreditumVisumEstAtCuadLongumEtCefallum.com' | ||
| it "should convert '#{LONG}' correctly" do | ||
| expect(Addressable::IDNA.to_unicode(LONG)).to eq(LONG) | ||
| long = 'AcinusFallumTrompetumNullunCreditumVisumEstAtCuadLongumEtCefallum.com' |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| LONG = 'AcinusFallumTrompetumNullunCreditumVisumEstAtCuadLongumEtCefallum.com' | ||
| it "should convert '#{LONG}' correctly" do | ||
| expect(Addressable::IDNA.to_ascii(LONG)).to eq(LONG) | ||
| long = 'AcinusFallumTrompetumNullunCreditumVisumEstAtCuadLongumEtCefallum.com' |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
|
Now that Ruby 2.5.0 has been released, I am seeing this regularly: Would be great to see this PR merged! |
|
I've created the PR to @amatsuda fork for resolving comments by @houndci-bot. |
| else | ||
| unpacked_result << starter | ||
| starter = ch | ||
| startercc = cc |
There was a problem hiding this comment.
Interesting. This was a straight port from C, so the original C probably had this unused variable too. Never noticed before. Cool.
|
Re: Hound, we need to go through the codebase and just update the code style across the board. Lots of PRs are going to get flagged like this until we do. |
|
There's also the |
|
@sporkmonger any chance there can be an official gem release that includes these changes? I've added a few gems that rely on |
|
@sporkmonger Can I ask you to cut a new release? |
|
Echoing @twelvelabs and @mtsmfm - pretty please can we have a new gem release with these changes, @sporkmonger? |
|
@sporkmonger Friendly nudge. It would be great to see a new, officially versioned released published with this fix. |
|
Hey, I know it's been awhile. I'm sorry about that. I should probably just set a recurring "do maintenance on addressable" calendar reminder for myself. Cutting a release right now. |
Here are fixes for some Ruby-level warnings.
The first two "unused variable" warnings happen under Ruby 2.5 trunk, and the last one can be seen under all versions of Ruby.