Zed A. Shaw
zedshaw at zedshaw.com
Thu Nov 22 13:10:43 EST 2007
On Thu, 22 Nov 2007 14:00:58 +0530
hemant <gethemant at gmail.com> wrote:
> There are two files which are there from eventmachine, deferable.rb
> and buftok.rb. Heck, there are just libs for aiding users.
> Packet doesn't depend on activesupport, It did depend initially, but i
> removed that thing altogether.
There was a comment at the top of a few files, like I said, 10 minutes cursory glance.
> I know, many people don't like,
> modifying core classes, and hence i am going to add them in helpers
> and it can potentially wreck havoc with other user code. I fully
> understand that. But lets see, how many methods were added to core
The issue isn't so much hacking the core classes, but rather that the primary project that would use your stuff (Rails) already does this. What happens when they change their monkey patching to be slightly different from your's? That'll cause incompatibilities, and since Ruby's open classes and mixins don't provide any warnings or versioning in this case of clashing-hackery you'd be screwing all the people who use your stuff.
It also would mess with just about any project that doesn't use or want to use Rails. What about the folks using Facets? I'm sure there's API differences in many of the monkey patching you do, Facets does, and Rails does.
In general, when you write a low level library like Packet that has to coexist with lots of other bad code, you have to keep it clean and pristine with a minimal of evil.
> iattr_accessor can't be removed, because it adds attribute accessors
> around class intance variables. If you see, its quite useful thing.
> Since, class instance attributes aren't shared across inheritance
Or, you can just design it to not need that. When I looked at it first thing I thought was it screamed of over design and cleverness, which just doesn't work in a project that has to coexist. Read above for more reasons why extending the base interactions of Ruby in your library is really a bad idea.
> It sure as hell needs work, whats more, this is my first ever public gem.
Yep, I get that, and my comments are no slight to you personally. But, if you ever want it to be adopted, then take my advice and stop doing fancy stuff. My comments were also more for the Mongrel team since they went rushing to your fresh gem too quickly. As I said, if a quick 10 minute glance can give me that many warning signs then it's not ready for use.
> Regarding licensing issues i don't have much to say. Code is there,
> take it or leave it. But I do not understand, packet is dual licensed,
> so effectively one can use it under Ruby license and forget the fact
> that its under GPL too, because choice of license is left to the user.
You're right, I missed that the EM code iss also licensed under the Ruby license and GPL2 similar to Mongrel.
But, a quick clarification:
> Also, mongrel itself is under GPL2, so how come two code bases, which
> are both under GPL can't use each others code? I am missing here
> something important .fetch me Eben. This is sure funny.
No, you're mistaken there, here's the line that mentions the GPL:
"You can redistribute it and/or modify it under either the terms of the GPL2 or the conditions below:"
This isn't saying it's licensed under GPL2, it's saying you can use either GPL2 -OR- the regular license. This is purely so that GPL purists can use the project, and everyone can not use the GPL if they want. It does NOT say that you have to use the GPL.
So for licensing you're good, but watch out for it. When you start borrowing code from other people you can get into trouble if they suddenly change their license, decide to interpret it differently, or just get pissy about how you use it.
The rest of my comments still stand though. Keep working on Packet, but my suggestion is for you to go back and make it cleaner, smaller, include less external code, rely on less, and don't do monkey patching so much.
Zed A. Shaw
- Hate: http://savingtheinternetwithhate.com/
- Good: http://www.zedshaw.com/
- Evil: http://yearofevil.com/
More information about the Mongrel-users