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.
149379873fe2cb70e550c6bff8fedd0c?d=identicon&s=25 Jeff Schwab (Guest)
on 2009-03-04 02: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
149379873fe2cb70e550c6bff8fedd0c?d=identicon&s=25 Jeff Schwab (Guest)
on 2009-03-04 21:15
(Received via mailing list)
Jeff Schwab 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
6883e5ef03484d4fcef507d7b4f1d243?d=identicon&s=25 Matt Jones (Guest)
on 2009-03-04 21:58
(Received via mailing list)
On Mar 4, 3:14 pm, Jeff Schwab <j...@schwabcenter.com> wrote:
> Jeff Schwab 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 Jones
40db9e75b3f5899258e3bdc0c9210154?d=identicon&s=25 Conrad Taylor (conradwt)
on 2009-03-04 22:14
(Received via mailing list)
On Wed, Mar 4, 2009 at 12:57 PM, Matt Jones <al2o3cr@gmail.com> wrote:

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

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

Good luck,

-Conrad
149379873fe2cb70e550c6bff8fedd0c?d=identicon&s=25 Jeff Schwab (Guest)
on 2009-03-04 22:54
(Received via mailing list)
Matt Jones wrote:
> On Mar 4, 3:14 pm, Jeff Schwab <j...@schwabcenter.com> 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.
149379873fe2cb70e550c6bff8fedd0c?d=identicon&s=25 Jeff Schwab (Guest)
on 2009-03-05 01:47
(Received via mailing list)
Jeff Schwab 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.
40db9e75b3f5899258e3bdc0c9210154?d=identicon&s=25 Conrad Taylor (conradwt)
on 2009-03-05 02:49
(Received via mailing list)
On Wed, Mar 4, 2009 at 4:25 PM, Jeff Schwab <jeff@schwabcenter.com>
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 Bates 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
6883e5ef03484d4fcef507d7b4f1d243?d=identicon&s=25 Matt Jones (Guest)
on 2009-03-05 20: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 Jones
40db9e75b3f5899258e3bdc0c9210154?d=identicon&s=25 Conrad Taylor (conradwt)
on 2009-03-05 22:08
(Received via mailing list)
On Thu, Mar 5, 2009 at 11:10 AM, Matt Jones <al2o3cr@gmail.com> 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
40db9e75b3f5899258e3bdc0c9210154?d=identicon&s=25 Conrad Taylor (conradwt)
on 2009-03-05 22:19
(Received via mailing list)
On Thu, Mar 5, 2009 at 1:07 PM, Conrad Taylor <conradwt@gmail.com>
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
149379873fe2cb70e550c6bff8fedd0c?d=identicon&s=25 Jeff Schwab (Guest)
on 2009-03-05 22:42
(Received via mailing list)
Matt Jones 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.