Code Reviews - aka "oh god... he's at it again..."

4 views
Skip to first unread message

Ryan Davis

unread,
Nov 28, 2011, 5:56:51 PM11/28/11
to RubyGems developers mailing list
I'd like to get rubygems moving again.

Specifically, rubygems codebase is a crufty mess and we need to fix it. First, we need to know what to fix. I'd like to see us do a full set of code reviews across the entire codebase with comment tags put in where the pain points are (detailed below). That way we can prioritize our pain and start to address it.

This effort is intended to go towards a 2.0 release, NOT a 1.x release. It is meant to rejuvenate the codebase and get us pointed in the right direction.

We've got 95 files in lib with ~20k lines in them. Every file should have at least one set of eyeballs on every line them with certain files having MANY sets of eyeballs on them. We've got 94 files in test with ~17k lines in them. They're just as important (if not more) as the files in lib.

I'd like to get people to step up and schedule code reviews over the next 2-4 weeks. Who's in? Let me know here that you're interested and how much you think you'd like to do.

## Tagging:

Nothing complex... I standardize on the following (examples):

# FIX: xxx is wrong
# HACK: should have done xxx instead
# TODO: xxx needs yyy
# DOC
# REFACTOR: duplicated from xxx
# RETIRE: replaced by xxx
# WARN: xxx is scary

I have a script that shows all of these in a clean way. Here is the summary of specification.rb:

Occurances:
32: TODO
2: FIX
1: REFACTOR
1: HACK

So you can use that as the basic idea that I'm going for.

_______________________________________________
RubyGems-Developers mailing list
http://rubyforge.org/projects/rubygems
RubyGems-...@rubyforge.org
http://rubyforge.org/mailman/listinfo/rubygems-developers

Nick Quaranto

unread,
Nov 28, 2011, 6:41:54 PM11/28/11
to RubyGems developers mailing list
May I suggest we do this in a branch instead of master? Just in case we
need to cut a new release and we can go bananas in a separate branch and
merge it in once we're happy.

Steve Klabnik

unread,
Nov 28, 2011, 8:30:06 PM11/28/11
to RubyGems developers mailing list
I've never looked at any of the RubyGems code, so if there's a part
that would particularly be helped out by an extra pair of brand new
eyes, I wouldn't mind pitching in a few hours.

Ryan Davis

unread,
Nov 28, 2011, 8:31:52 PM11/28/11
to RubyGems developers mailing list

On Nov 28, 2011, at 15:41 , Nick Quaranto wrote:

> May I suggest we do this in a branch instead of master? Just in case we
> need to cut a new release and we can go bananas in a separate branch and
> merge it in once we're happy.

We're currently doing the opposite. 1.8.x is released from the 1.8 branch.

Luis Lavena

unread,
Nov 28, 2011, 8:55:28 PM11/28/11
to RubyGems developers mailing list
On Mon, Nov 28, 2011 at 7:56 PM, Ryan Davis <ryand...@zenspider.com> wrote:
> I'd like to get rubygems moving again.
>

/me wants a better remote fetcher, perhaps API driven one instead of
massive marshalled data?

--
Luis Lavena
AREA 17
-
Perfection in design is achieved not when there is nothing more to add,
but rather when there is nothing more to take away.
Antoine de Saint-Exupéry

Ryan Davis

unread,
Nov 29, 2011, 5:36:17 AM11/29/11
to RubyGems developers mailing list

On Nov 28, 2011, at 17:55 , Luis Lavena wrote:

> On Mon, Nov 28, 2011 at 7:56 PM, Ryan Davis <ryand...@zenspider.com> wrote:
>> I'd like to get rubygems moving again.
>>
>
> /me wants a better remote fetcher, perhaps API driven one instead of
> massive marshalled data?

/me too :P

Can you make sure there is a ticket for that?

Ryan Davis

unread,
Nov 29, 2011, 5:40:10 AM11/29/11
to RubyGems developers mailing list

On Nov 28, 2011, at 17:30 , Steve Klabnik wrote:

> I've never looked at any of the RubyGems code, so if there's a part
> that would particularly be helped out by an extra pair of brand new
> eyes, I wouldn't mind pitching in a few hours.

Awesome Steve. Thanks!

Any amount of eyeballs that we can get on rubygems.rb and specification.rb will help. Granted, there's plenty more too... but those two files contain most of the functionality/risk/ugly.

I'd like to continue slimming down rubygems.rb because it has been a grab-bag of crap for many years now. Functionality in there should move the the actual responsible class. Most of the work I did this year was moving stuff from there (and other places) to specification since most of the work of rubygems has to do with specifications.

Reply all
Reply to author
Forward
0 new messages