[Ironruby-core] Mapping multiple pushes to a single review
Jimmy.Schementi at microsoft.com
Mon Mar 30 21:19:35 EDT 2009
As a slight variation/summarization, I'd suggest making a branch for any specific change you'll be making, especially if it'll be made up of multiple commits. Pulls could be made on master, and merged into the branch as needed. Then, you could rebase and present the change as one commit to your master branch, which then could go out for code review.
Though, I liked having many small changes, since the modified-file-count was small per link, so it was almost like having a file list. Though github makes it a bit irritating that it doesn't show the diffs of new or deleted files, so you have to click on each one of those.
From: ironruby-core-bounces at rubyforge.org [mailto:ironruby-core-bounces at rubyforge.org] On Behalf Of Shri Borde
Sent: Monday, March 30, 2009 4:44 PM
To: Jim Deville
Cc: ironruby-core at rubyforge.org
Subject: [Ironruby-core] Mapping multiple pushes to a single review
Could you have avoided doing pulls from the main repo? Not ideal but given the tools we have, we will have to figure out a solution that balances all interests.
You could have a separate branch for active work if you do need pulls for main for some other reason like doing code reviews for others, just keeping on top of whats going on, etc.
Also, if you can manage to avoid pulls from the main repo, should rebasing be recommended? You want to do frequent pushes maily to back up your changes in the cloud, and so rebasing sounds like a good option once you are finally happy with the changes and want to push it into your master branch. It will be easier to browse file histories...
Also, I suppose this issue is not unique to IronRuby. How do other projects deal with code reviews, keeping file history less noisy, etc?
From: Jim Deville
Sent: Friday, March 27, 2009 11:40 AM
To: IronRuby External Code Reviewers
Cc: ironruby-core at rubyforge.org
Subject: Code Review: Various .NET interop tests
This is one of the weak areas for our Git setup. I have multiple commits that need review. I honestly feel that Git needs the concept of a push object, so I could say look at this, or these pushes. So, options to help you review:
* Click on each of these links and leave comments
* Use git however you want
* Use Git Extensions (if you clone git://github.com/jredville/ironruby.git, and setup Git Extensions, you should be able to click on each commit, and each file at your leisure)
* GitDiff 17de3f82 8a54299 (Unfortuneately, this will include commits and files from IronRuby's main repo's pushes)
We're trying to figure something better out, including possibly using Review Board (http://review-board.org). So, now for the actual review:
This a some work on Generic Method specs, as well as more general work on a first pass of the test plan (http://ironruby.net/.NET_Testing_plans).
* modify .gitignore to ignore Bin in addition to bin<http://github.com/jredville/ironruby/commit/f1ae28a5810c2c985984509a8cd75426170febd2>
* add dlr_config to IronRuby object<http://github.com/jredville/ironruby/commit/b210faca750b08a41aa8667509ac1560dba5352a>
* csc describe handles shared specs (multiple arguments) and we now emit <http://github.com/jredville/ironruby/commit/c863df5295a5b273cd9bc55b04388473a924e718>
#line pragmas instead of comments<http://github.com/jredville/ironruby/commit/c863df5295a5b273cd9bc55b04388473a924e718>
* adding some generic tests and fixing tags, also make default.mspec load <http://github.com/jredville/ironruby/commit/33916da95ffd1d3edc372cc018244506c40f6d76>
ir.exe instead of ir.cmd<http://github.com/jredville/ironruby/commit/33916da95ffd1d3edc372cc018244506c40f6d76>
* adding some constrained generic specs<http://github.com/jredville/ironruby/commit/2e151df2150776ee536518c79b9e150045c697f3>
* added generic error messages specs. Fixed tag locations<http://github.com/jredville/ironruby/commit/b0f28633e2bad7ad64adfa670a185efcb1883160>
* split pragma warning to make sure I do not disable unintended warnings. <http://github.com/jredville/ironruby/commit/8167bd51b5c5a1665a56849df520d83b015fc838>
Refactor conflicting methods<http://github.com/jredville/ironruby/commit/8167bd51b5c5a1665a56849df520d83b015fc838>
* added class param and conflicting type param specs<http://github.com/jredville/ironruby/commit/4819985516558b47819caf5b3979ca25a017ff56>
* adding specs for ruby classes with type constraints and for a secondary <http://github.com/jredville/ironruby/commit/9f40c77354aaba6930ef968bb40df26ea6ae629c>
(base class) type constraint)<http://github.com/jredville/ironruby/commit/9f40c77354aaba6930ef968bb40df26ea6ae629c>
* array conversion specs<http://github.com/jredville/ironruby/commit/f47f33b057afac0e99b92dbfc557fe67fbb2b46c>
* array instantiation specs<http://github.com/jredville/ironruby/commit/183983621076f7bb4f45e7c9c22118e3b3e2f252>
* redid IronRuby.dlr_config after Tomas' IronRuby changes<http://github.com/jredville/ironruby/commit/a446fbd0bb2c0a5788ee452f8a784d77ce5bd39c>
* adding a default conversion spec and a little bit of refactoring<http://github.com/jredville/ironruby/commit/b43afd9f71cd5273f1d9b3f777f620812ee2af58>
* more array tests<http://github.com/jredville/ironruby/commit/600a3aed26f21c092d2c9efccbd722d0c7a8bb3a>
* spec for a static method caching bug i found<http://github.com/jredville/ironruby/commit/2c7f69786a4c6afb852bd2bfe86af42c74ae3ee6>
* spec method overriding maintains .NET supermethod<http://github.com/jredville/ironruby/commit/82e5a3bad6cd0fa0a9b9572730007aa22ef05a43>
* refactor to add some metaclass helpers<http://github.com/jredville/ironruby/commit/4c9d1df91350db6a82e3f26c10fb073fee00cc53>
* class instantiation specs<http://github.com/jredville/ironruby/commit/fa6de43521f4f87964dde49a100787f9325f419f>
* some more class instantiation specs<http://github.com/jredville/ironruby/commit/42b0882d0cd6297378060591af17f77f79067936>
* sealed class instantiation specs<http://github.com/jredville/ironruby/commit/21eaf4cae6b73c9ae7a6cdc565a0faf17f770220>
* generic instantiation specs<http://github.com/jredville/ironruby/commit/5e8cf7298067a9d80fb5806ad9fa2ac7a0f76878>
* make GenericClass have a method so it isn't EmptyGenericClass<http://github.com/jredville/ironruby/commit/47b05ed4b6a01cdc501b2514c91a2744d06762f0>
* more generic instantiation specs<http://github.com/jredville/ironruby/commit/8a54299a822d8bde5451ccb91d5d1545a5aa1778>
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the Ironruby-core