Forum: Ruby on Rails submit a patch?

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
Jeff S. (Guest)
on 2009-03-04 03:59
(Received via mailing list)
Short version:  How do I submit a patch?

Long version:
I hit an error with Rails 2.3 on Ruby 1.9.1, debugged it, and fixed it
locally.  Now, I'd like to open a bug report and submit a patch, but I'm
having a hard time figuring out how.

I'm watching a RailsCast, but every little step is taking hours.  The
lighthouse site was down (but is back up now), I didn't have (and had
never used) git, I just spent an hour getting mysql [1] installed (since
the RailsCast claims it's required by rails self-test suite), and now
rails test is failing with a bug that somebody else apparently reported
yesterday against Netbeans (which I'm not using). [2]

1. Btw, why doesn't mysql on OS X read ~/.inputrc?  GNU readline is
installed.

2. http://forums.netbeans.org/post-26407.html
Jeff S. (Guest)
on 2009-03-04 22:15
(Received via mailing list)
Jeff S. wrote:
> Short version:  How do I submit a patch?

Submitted ticket 2130, and attached the patch.
http://rails.lighthouseapp.com/projects/8994-ruby-...

If anybody here knows what they're doing and would care to close the
ticket, please be my guest.

$ svn diff vendor/rails/railties/lib/commands/plugin.rb
Index: vendor/rails/railties/lib/commands/plugin.rb
===================================================================
--- vendor/rails/railties/lib/commands/plugin.rb        (revision 11)
+++ vendor/rails/railties/lib/commands/plugin.rb        (working copy)
@@ -134,7 +134,7 @@
    def externals
      return [] unless use_externals?
      ext = `svn propget svn:externals "#{root}/vendor/plugins"`
-    ext.reject{ |line| line.strip == '' }.map do |line|
+    ext.lines.reject{ |line| line.strip == '' }.map do |line|
        line.strip.split(/\s+/, 2)
      end
    end
Matt J. (Guest)
on 2009-03-04 22:58
(Received via mailing list)
On Mar 4, 3:14 pm, Jeff S. <removed_email_address@domain.invalid> wrote:
> Jeff S. wrote:
> > Short version:  How do I submit a patch?
>
> Submitted ticket 2130, and attached the 
patch.http://rails.lighthouseapp.com/projects/8994-ruby-...
>
> If anybody here knows what they're doing and would care to close the
> ticket, please be my guest.
>

There are a few technical considerations to look at, which I've added
to #2130.
However, on a procedural note, Git is pretty much required to submit
patches.

Good start on the patch - thanks!

--Matt J.
Conrad T. (Guest)
on 2009-03-04 23:14
(Received via mailing list)
On Wed, Mar 4, 2009 at 12:57 PM, Matt J. <removed_email_address@domain.invalid> 
wrote:

> > If anybody here knows what they're doing and would care to close the
> --Matt J.
>
>
Hi, I would recommend the following railscasts:

Contributing to Rails with
Git<http://railscasts.com/episodes/113-contributing-to...

Good luck,

-Conrad
Jeff S. (Guest)
on 2009-03-04 23:54
(Received via mailing list)
Matt J. wrote:
> On Mar 4, 3:14 pm, Jeff S. <removed_email_address@domain.invalid> wrote:
>> Submitted ticket 2130, and attached the patch.

> There are a few technical considerations to look at, which I've added
> to #2130.
> However, on a procedural note, Git is pretty much required to submit
> patches.

Understood.  That's my next step.  I've been getting bitten by
"technical considerations" on that front, too, but I'm hoping to have it
working later today.
Jeff S. (Guest)
on 2009-03-05 02:47
(Received via mailing list)
Jeff S. wrote:
> "technical considerations" on that front, too, but I'm hoping to have it
> working later today.

What's the right way to test this fix?  I don't see any tests for class
RailsEnvironment.
Conrad T. (Guest)
on 2009-03-05 03:49
(Received via mailing list)
On Wed, Mar 4, 2009 at 4:25 PM, Jeff S. <removed_email_address@domain.invalid>
wrote:

> >
> > Understood.  That's my next step.  I've been getting bitten by
> > "technical considerations" on that front, too, but I'm hoping to have it
> > working later today.
>
> What's the right way to test this fix?  I don't see any tests for class
> RailsEnvironment.
>

Hi, I really liked Ryan B. approach by writing the test first to
verify
that
his enhancement doesn't exist and fails.  It really depends on what you
fixed
and how.

Good luck,

-Conrad
Matt J. (Guest)
on 2009-03-05 21:11
(Received via mailing list)
I'd argue that since this is essentially just a Ruby 1.9 compatibility
thing, that it shouldn't really need testing. Especially if you adopt
the approach used earlier in the same file.

--Matt J.
Conrad T. (Guest)
on 2009-03-05 23:08
(Received via mailing list)
On Thu, Mar 5, 2009 at 11:10 AM, Matt J. <removed_email_address@domain.invalid> 
wrote:

>
> I'd argue that since this is essentially just a Ruby 1.9 compatibility
> thing, that it shouldn't really need testing. Especially if you adopt
> the approach used earlier in the same file.
>

If and only if, the test already exists.

-Conrad
Conrad T. (Guest)
on 2009-03-05 23:19
(Received via mailing list)
On Thu, Mar 5, 2009 at 1:07 PM, Conrad T. <removed_email_address@domain.invalid>
wrote:

> -Conrad
>

I jump the gun on the send button.  Anyway, I feel that all enhancements
and
bug
fixes should have an associated test that validates the fix or
enhancement
with
even in the case where it happens to be Ruby 1.9 compatibility.  Why?
If
you fix
something and/or add a feature without a test, one wouldn't be able to
verify that it
works from release to release.  Furthermore, I would rather catch a
development
error in development rather than production.

Just my 2 cents,

-Conrad
Jeff S. (Guest)
on 2009-03-05 23:42
(Received via mailing list)
Matt J. wrote:
> I'd argue that since this is essentially just a Ruby 1.9 compatibility
> thing, that it shouldn't really need testing.

Agreed, although a smoke test on 1.8 would do my heart good.  How does
that sort of thing usually work?  Are there volunteer alpha testers, or
nightly regression tests, running in different environments, with
different versions of Ruby?

> Especially if you adopt
> the approach used earlier in the same file.

I'm not sure what you mean here.  The relevant class, RailsEnvironment,
is the first in the file (plugin.rb), and the file does not contain any
mention of testing.  There is a PluginTest class in plugin_test.rb, but
it doesn't directly test RailsEnvironment.

The RailsEnvironment class depends inherently on the caller's
environment, so this is kind of a special case.  For the failure to
happen, there has to be a .svn directory in "#{root}/vendor/plugins",
and backtick calls to svn must work.  If the user hasn't got svn
installed, should we mock out a stub just for this test?  I see that
PluginTest already uses some stubbing tools, but I'm not familiar with
them.

I'm guessing that for a Ruby newbie like myself, adding the test is
probably about a day's effort, and the code may not be as, uh,
aesthetically pleasing as one would like.  I'll give it a shot, anyway.
This topic is locked and can not be replied to.