Code Reviews

Ryan Davis ryand-ruby at zenspider.com
Mon Dec 12 21:09:20 EST 2011


Hi there!

As discussed on the rubygems-dev mailing-list a couple weeks ago, 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.

I'd like you to do a code review of rubygems specifically focusing on the following files:

    Aaron Patterson:

      lib/rubygems/command.rb
      lib/rubygems/commands/*.rb

    Eric Hodel:

      lib/rubygems/builder.rb
      lib/rubygems/command_manager.rb
      lib/rubygems/config_file.rb
      lib/rubygems/custom_require.rb
      lib/rubygems/defaults.rb
      lib/rubygems/dependency_list.rb
      lib/rubygems/deprecate.rb
      lib/rubygems/doc_manager.rb
      lib/rubygems/errors.rb
      lib/rubygems/exceptions.rb
      lib/rubygems/format.rb
      lib/rubygems/gem_openssl.rb
      lib/rubygems/gem_path_searcher.rb
      lib/rubygems/gem_runner.rb
      lib/rubygems/gemcutter_utilities.rb
      lib/rubygems/indexer.rb
      lib/rubygems/install_message.rb
      lib/rubygems/install_update_options.rb
      lib/rubygems/installer_test_case.rb

    Ryan Davis:

      lib/rubygems/local_remote_options.rb
      lib/rubygems/mock_gem_ui.rb
      lib/rubygems/old_format.rb
      lib/rubygems/package_task.rb
      lib/rubygems/path_support.rb
      lib/rubygems/platform.rb
      lib/rubygems/rdoc.rb
      lib/rubygems/remote_fetcher.rb
      lib/rubygems/require_paths_builder.rb
      lib/rubygems/security.rb
      lib/rubygems/server.rb
      lib/rubygems/source_index.rb
      lib/rubygems/spec_fetcher.rb
      lib/rubygems/test_case.rb
      lib/rubygems/test_utilities.rb
      lib/rubygems/text.rb
      lib/rubygems/user_interaction.rb
      lib/rubygems/validator.rb
      lib/rubygems/version_option.rb

    Evan Phoenix:

      lib/rubygems/dependency_installer.rb
      lib/rubygems/installer.rb
      lib/rubygems/uninstaller.rb
      lib/rubygems/ext/builder.rb
      lib/rubygems/ext/configure_builder.rb
      lib/rubygems/ext/ext_conf_builder.rb
      lib/rubygems/ext/rake_builder.rb
      lib/rubygems/ext.rb

    Steven Baker:

      lib/rubygems.rb
      lib/rubygems/specification.rb
      lib/rubygems/version.rb
      lib/rubygems/requirement.rb
      lib/rubygems/dependency.rb

    Steve Klabnik:

      lib/rubygems.rb
      lib/rubygems/specification.rb
      lib/rubygems/version.rb
      lib/rubygems/requirement.rb
      lib/rubygems/dependency.rb

Secondarily, anything on the following list needs as many eyeballs as we can get:

    lib/rubygems.rb
    lib/rubygems/specification.rb
    lib/rubygems/installer.rb

Tertiarily, I'd like to get eyeballs on these:

    test/**/*.rb

## Timeframe:

I'd like to get these reviews done in the next week or two. Please let me know if that is an issue for you so I can juggle assignments.

## Submitting your review:

If you don't have commit bit already, just fork rubygems and submit a pull request to me. If you do have commit bit, please commit to master.

If you find overarching issues with the code that you don't think is appropriate as a code comment (eg, all option related files should be namespaced under rubygems/opt/*.rb), feel free to create a new issue on github instead.

## 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



More information about the RubyGems-Developers mailing list