Date: 2009-08-18 15:46
Sender: Tom Copeland
Hi 7rans -
Will reply more later, but just FYI, I had "7rans"
in the beta list - I've changed that to "transami"
so that you can use that account instead.
Yours,
Tom
|
Date: 2009-08-17 15:58
Sender: 7rans
Hi--
Looked over the code this afternoon. I'm glad to see this is
coming along.
I have one significant issue I'd like to see addressed. I'd like
to see an extra layer of code separation in the design. Right
now the calls are tightly tied into how rubyforge qua execuable
handles configuration. In other words I'd like to see a clean
functional layer below the current one --something others can
use to easily interact with Rubyforge if they wish to do so in
a way other then how the rubyforge program does.
Let me give an specific example to be clear. In the code:
def add_release(group_id, package_id, release_name, *files)
group_id = lookup "group", group_id
package_id = lookup "package", package_id
release_date = @userconfig["release_date"]
release_notes = @userconfig["release_notes"]
release_changes = @userconfig["release_changes"]
preformatted = @userconfig["preformatted"]
...
Notice the settings release_* are being pulled from @userconfig.
This function ends by calling #run. In #run we see:
def run(page, form, extheader={}) # :nodoc:
uri = self.uri + page
puts "client.post_content #{uri.inspect}, #{form.inspect},
#{extheader.inspect}" if $DEBUG
response = client.post_content uri, form, extheader,
@userconfig
puts response if $DEBUG
response
end
And here we see the whole of @userconfig being passed to the
backend client. This is tight coupling, as well as redundant.
It would be cleaner to create a completely separate layer as
a superclass of the current one that defines something like:
def add_release(group_id, package_id, release_name, config,
*files)
group_id = group_id
package_id = package_id
release_date = config["release_date"]
release_notes = config["release_notes"]
release_changes = config["release_changes"]
preformatted = config["preformatted"]
...
And likewise for #run, that does not depend on any global config,
but rather must receive all relevant parameters via the method
interface.
The separation should be clean enough that conceivably we could
create a 'rubyforge-api' gem that the 'rubyforge' gem depends
on (though separate gems are not necessary, I'm just clarifying
the point about the clean code separation).
HTH. Let me know if you would like me to put a little elbow grease
behind this idea (I would need some help with certain details,
but I could certainly do the bulk of the work).
|
