Untitled

[trans]

No description provided.

[luislavena]

Hello Trans,

Can you better explain the rationale of this addition? You did provide a test or documentation for this, nor was discussed in the mailing list.

Thank you.

[drbrain]

There is no recommended usage documentation provided. Providing a big bucket to dump arbitrary data in does not lend itself well to careful and thoughtful usage by gem developers.

Most of the per-method documentation is missing.

If this is supposed to be a URL-store why aren't entries validated by URI?

Gem::Resource

I do not understand why both attr_accessor is overridden and method_missing is provided. method_missing appears to be the better solution as it has the smallest implementation.

Gem::Resource#to_h will never be called as Gem::Specification#ruby_code is never called with a Gem::Resource. Furthermore, I don't understand why it isn't implemented as "return @table.dup".

Gem::Resource#home/#homepage duplicates an attribute provided on Gem::Specification.

I don't understand why multiple aliases are provided.

Use $` instead of converting label to a string multiple times and chomping in #method_missing

If you're not going to use the block in #method_missing there's no need to capture it in an argument.

Why are #hash and #eql? implemented? Will Gem::Resource be used as a Hash key? I cannot conceive of this.

Why is a protected reader for @table provided? This class doesn't make use of it.

Gem::Resource#set is a useless abstraction

It appears that a resources Hash on Gem::Specification would be a better solution. This implementation requires triple-dispatch. (Gem::Specification#add_resource => Gem::Resource#add_resource => #resource_name => Hash#[]= vs Gem::Specification#add_resource => Hash#[]=)

Gem::Specification

Resources MUST NOT be included in the Marshal gemspec. Since #resources stores arbitrary data it presents a risk of high bandwidth usage through user's ignorance. (Furthermore, if the Marshal format is altered the specification version MUST be changed.)

Why is Gem::Resource handled in Gem::Specification#ruby_code but never used?

Why is Gem::Specification#add_resource aliased to Gem::Specification#resource? #resource is one letter different from #resources but does something entirely different.

There is no protection around #add_resource in the generated gemspec for earlier RubyGems versions. (Yes, people will install gems with a version of RubyGems that supports #add_resource then attempt to use them on a version that does not.)

[trans]

Many of your questions can be answered by two points.

1. "I don't understand why multiple aliases are provided." Aliases are provided for the the convenience of using a shorter term, eg. mail vs. mailing_list. The short terms also line up nicely making it easier to read write. The longer terms of course correspond the terms used on rubygems.org. B/c of the aliases, a separate class is called for rather then just a hash, as is the attr_accessor and not just method_missing. In junction with this I would not bet against other resources might exist unaccounted for by a fixed spec, so the ability to add additional arbitrary entries was made possible. (I don't see why anyone would abuse it any more than anything else can already be abused.)

2. This is not a 100% complete implementation. I am well aware some version conditions need to added, but not knowing what version it will be I left that to be done. As I am well aware other details will need consideration too, e.g. URL validation. The marshaling and ruby_code part were a bit confusing to me, I worked them as best as I could make out and get the current tests to pass. Things like #hash/eql? were implemented b/c the tests actually would not pass without it. And obviously there is a Gem::Resource#home/#homepage duplication of an attribute provided on Gem::Specification, ultimately they would be refactored to reference the same data.

I think the rest of your comments are merely minor details, which can be adjusted as seen fit, of course.

[drbrain]

One descriptive, concise name is better than four aliases as it removes any need for an extra class and is easier to document.

You don't need a version check, #respond_to? is sufficient. Gem::Specification#to_ruby already makes use of this.

Your tests do not require Gem::Resources#hash nor #eql?

If this isn't a complete implementation and ready for inclusion in RubyGems why did you create a pull request?

[drbrain]

I ran all the tests with your patch applied and without hash/eql?. Nothing failed.

If you are looking for feedback on an idea you should go to the mailing list. When your idea is completed to the best of your ability you should submit a pull request for final review.

[drbrain]

Then you have some other bug. #eql? is aliased to #== which you did not modify to check for resource equality.

[zenspider]

5:12:46 PM zenspider: maybe I should start meta commenting and pointing out that
                      for nearly every single critique, he defended instead of
                      embraced / adapted
5:13:10 PM Professor: perhaps
5:13:14 PM zenspider: and that RANTY TIRADES usually mean you're done because 
                      you've blown all legitimacy with your audience if you 
                      come back 10 minutes later
5:13:17 PM Professor: he tried to defend every single one
5:13:26 PM Professor: none of them he went "ok, thats fine, i'll do it that way"
5:13:28 PM zenspider: he needs to learn how the fuck to take critiques
5:13:45 PM Professor: yep