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

Add 'broadcast' method to IPAddress::IPv6 #31

Closed
wants to merge 1 commit into from

Conversation

gdlx
Copy link
Contributor

@gdlx gdlx commented Mar 8, 2013

broadcast method was missing in IPAddress::IPv6.

I needed it because I store networks broadcast address in DB (this way I can find network hosts by searching ones between network and broadcast addresses).

@gdlx
Copy link
Contributor Author

gdlx commented Sep 3, 2013

Does someone manage this repo ?

@t3hk0d3
Copy link

t3hk0d3 commented Sep 3, 2013

Unfortunately author abandoned this project :(

I think community should fork and claim this gem on rubygems.

@gdlx
Copy link
Contributor Author

gdlx commented Sep 3, 2013

Thanks, but only the author commited to the repo so it's not easy to know who could take it over...

@bluemonk
Copy link
Collaborator

bluemonk commented Sep 3, 2013

Hello guys,

I'm still here. I kind of lost interest in the project when the proposal to
replace IPAddr in the standard library was rejected a year ago, but I've
seen an increase of engagement in the last few months and I'm considering
coming back to actively maintain it.

On Tue, Sep 3, 2013 at 4:44 PM, Igor Yamolov [email protected]:

Unfortunately author is abandoned this project :(

I think community should fork and claim this gem on rubygems.


Reply to this email directly or view it on GitHubhttps://github.com//pull/31#issuecomment-23717765
.

@gdlx
Copy link
Contributor Author

gdlx commented Sep 3, 2013

@bluemonk Not being in the standard library doesn't make your gem useless (I actually prefer chossing y gems than having a fat standard library full of things I don't use).

I think the increase is due to the growing need of IPv6 (look at most recent pull requests).

@Spaceghost
Copy link

@bluemonk I think that being part of the stdlib is a neat idea, but having a great gem for working with IPAddresses is really valuable regardless of whether I get it in the standard distribution of ruby.

I've been involved in working with ip addresses in ruby extensively before, and we ended up writing a library in C that was a lot lighter and faster which we pulled back into ruby. This library is pretty awesome and I hope you know that even though you haven't released in a number of years, you're getting a lot of downloads. 2,379,792 to be exact.

Do you perhaps want a co-maintainer to help?

@francisluong
Copy link
Contributor

I've been using your gem at Salesforce.com, @bluemonk. It's good stuff.

Maybe we could organized the interested contributors on this thread into a review team for PRs. Could establish a cadence to incorporate 1-3 PRs a month (or review them at least and ask contributors to tighten/test sections better. I'd be glad to be part of this sort of effort.

@gdlx
Copy link
Contributor Author

gdlx commented Mar 7, 2016

3 years since this PR... some are more than 4 years old. Maybe it's time to allow other people to maintain this gem ?

@bluemonk
Copy link
Collaborator

bluemonk commented Mar 7, 2016

Absolutely. I've been given full admin access to this repo to anyone who asked for it. Mike Mackintosh for example. Let me know if you're interested.

@francisluong that's a great idea, let me know if you're still on it and I will add you immediately.

@bluemonk
Copy link
Collaborator

bluemonk commented Mar 7, 2016

On a side note, there's no broadcast concept in IPv6, which is why the gem is missing that particular method.

@francisluong
Copy link
Contributor

I wouldn't mind contributing.

If you can believe it, I hadn't known that there isn't a broadcast address
equivalent in IPv6. I was planning to use it to keep my code consistent
for grabbing the two addresses in a /31 and a /127.

If it's going to spread more confusion to have a broadcast method perhaps
it'd be better if we didn't incorporate such a change.

-Franco

On Monday, March 7, 2016, Marco Ceresa [email protected] wrote:

On a side note, there's no broadcast concept in IPv6, which is why the gem
is missing that particular method.


Reply to this email directly or view it on GitHub
#31 (comment).

[=- @francisluong https://twitter.com/FrancisLuong // AKA Franco :: *
http://www.francisluong.com
-=]*

@bluemonk
Copy link
Collaborator

bluemonk commented Mar 7, 2016

Yes, I'm against a method called broadcast for IPv6, but I don't mind a method with similar functionalities and another name (IPv6#anycast perhaps?).
Added you @francisluong

@ghg
Copy link
Contributor

ghg commented Mar 7, 2016

Another way would be to define broadcast on both, but have IPv6 raise an exception describing why it is unsupported. This might save a user that isnt as familiar with it some time (and also some duplicate issues being raised).

@mikemackintosh
Copy link
Collaborator

There is a similar issue on this, which I was waiting on some sort of feedback before dropping the hammer.

#54 (comment)

It would give insight into the different IPv6 categories:

I'd like to solicit some feedback in regards to having methods like #is_ula? and if it would be beneficial? Playing off this, we can also do #is_teredo?, #is_6to4?, #is_orchid?, #is_doc?, etc..

@gdlx
Copy link
Contributor Author

gdlx commented Mar 9, 2016

On a side note, there's no broadcast concept in IPv6, which is why the gem is missing that particular method.

@bluemonk You're absolutely right. I'm actually using this method to get the last address of a network, not for actually broadcasting stuff... I think I've said "broadcast" for the last address for so many years that it became a generic name in my mind...

That said, a method to get the last address is still useful, and note that the broadcast_u128 method I used was there prior to my PR!

I can rename the method to last, but keeping the same name than v4 is better for consistency IMHO.

@gdlx
Copy link
Contributor Author

gdlx commented Mar 9, 2016

My concern about review/merge delay was also about my other PR (#60 / "only" 1 year old) and other PRs. I just sent my ping in my older one.

As I've seen you've merged a PR recently, I thought it could be time to take a look at the 15 dusty other ones...

@smortex
Copy link
Contributor

smortex commented Aug 30, 2017

Closing this since since #broadcast is an IPv6 nonsense and the discussion digressed a lot (if people are interested in maintainership, please contact @francisluong).

If this features is still desired, maybe adding common IPv4#last and IPv6#last methods and aliasing this on the IPv4 class to broadcast may make sense.

@smortex smortex closed this Aug 30, 2017
@gdlx
Copy link
Contributor Author

gdlx commented Aug 30, 2017

I agree #last would have more sense than #broadcast, but it requires to add it to v4 too, for consistency.
As explained in #60, I'll do it if I find the time (this one is 4 years old 🤣)

TheHolyRoger pushed a commit to whatagem/ipaddress that referenced this pull request Oct 18, 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.

8 participants