* [PATCH] Adds pathname support [not found] <CALO9HP2oBDPBaeKx8N_Lp9GbfTOoGTFkKXcyfZJaJm=E_ksLOA@mail.gmail.com> @ 2012-06-05 0:07 ` Brian Corrigan 2012-06-05 0:47 ` Eric Wong 0 siblings, 1 reply; 3+ messages in thread From: Brian Corrigan @ 2012-06-05 0:07 UTC (permalink / raw) To: raindrops [-- Attachment #1: Type: text/plain, Size: 195 bytes --] Hi Guys - I've never submitted a patch via email before. Apologies if I get this completely wrong (go easy!) Also, I had a problem getting the tests to run on 1.9.3. In any case, here goes.. [-- Attachment #2: Type: text/plain, Size: 203 bytes --] Hi Guys - I've never submitted a patch via email before. Apologies if I get this completely wrong (go easy!) Also, I had a problem getting the tests to run on 1.9.3. In any case, here goes.. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Adds pathname support 2012-06-05 0:07 ` [PATCH] Adds pathname support Brian Corrigan @ 2012-06-05 0:47 ` Eric Wong 2012-06-05 15:45 ` Brian Corrigan 0 siblings, 1 reply; 3+ messages in thread From: Eric Wong @ 2012-06-05 0:47 UTC (permalink / raw) To: raindrops Brian Corrigan <bcorrigan78@gmail.com> wrote: > Hi Guys - I've never submitted a patch via email before. Apologies if > I get this completely wrong (go easy!) Thanks. I prefer patches inline since it's easier to apply/review, but some mail clients mangle them. git send-email comes with git and is a good way to send email if you're stuck on a mail client than mangles email. Also, don't include HTML parts/attachments to your email (ever :P). > Also, I had a problem getting > the tests to run on 1.9.3. In any case, here goes.. What was the error? 1.9.3 is my primary platform. I've inlined your patch below and have a few comments inline: > From fd82f8f1e0d8fa91b90235a066d1c5ad936cbc0d Mon Sep 17 00:00:00 2001 > From: Brian Corrigan <brian@agoragames.com> > Date: Mon, 4 Jun 2012 16:50:07 -0400 > Subject: [PATCH 1/2] Adds pathname support The commit message should explain why the change is necessary. > --- a/lib/raindrops/linux.rb > +++ b/lib/raindrops/linux.rb No require for 'pathname'? > @@ -41,6 +41,7 @@ module Raindrops::Linux > else > paths = paths.map do |path| > path = path.dup > + path = Pathname.new(path).realpath.to_s It should be possible to safely remove the "path = path.dup" line above. > path.force_encoding(Encoding::BINARY) if defined?(Encoding) > rv[path] > Regexp.escape(path) > diff --git a/test/test_linux.rb b/test/test_linux.rb > index 65f25e0..953549e 100644 > --- a/test/test_linux.rb > +++ b/test/test_linux.rb > @@ -1,6 +1,7 @@ > # -*- encoding: binary -*- > require 'test/unit' > require 'tempfile' > +require 'tmpdir' > require 'raindrops' > require 'socket' > require 'pp' I noticed your second patch reverted this change. Better to use "git commit --amend" to rewrite changes (especially trivial ones) before sending them so you don't propagate the noise going forward. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Adds pathname support 2012-06-05 0:47 ` Eric Wong @ 2012-06-05 15:45 ` Brian Corrigan 0 siblings, 0 replies; 3+ messages in thread From: Brian Corrigan @ 2012-06-05 15:45 UTC (permalink / raw) To: raindrops Eric - Let's try this again with a new thread/ Thanks for the tips!! On Mon, Jun 4, 2012 at 8:47 PM, Eric Wong <normalperson@yhbt.net> wrote: > Brian Corrigan <bcorrigan78@gmail.com> wrote: >> Hi Guys - I've never submitted a patch via email before. Apologies if >> I get this completely wrong (go easy!) > > Thanks. > > I prefer patches inline since it's easier to apply/review, but some mail > clients mangle them. git send-email comes with git and is a good way > to send email if you're stuck on a mail client than mangles email. > > Also, don't include HTML parts/attachments to your email (ever :P). > >> Also, I had a problem getting >> the tests to run on 1.9.3. In any case, here goes.. > > What was the error? 1.9.3 is my primary platform. > > > I've inlined your patch below and have a few comments inline: > >> From fd82f8f1e0d8fa91b90235a066d1c5ad936cbc0d Mon Sep 17 00:00:00 2001 >> From: Brian Corrigan <brian@agoragames.com> >> Date: Mon, 4 Jun 2012 16:50:07 -0400 >> Subject: [PATCH 1/2] Adds pathname support > > The commit message should explain why the change is necessary. > >> --- a/lib/raindrops/linux.rb >> +++ b/lib/raindrops/linux.rb > > No require for 'pathname'? > >> @@ -41,6 +41,7 @@ module Raindrops::Linux >> else >> paths = paths.map do |path| >> path = path.dup >> + path = Pathname.new(path).realpath.to_s > > It should be possible to safely remove the "path = path.dup" line above. > >> path.force_encoding(Encoding::BINARY) if defined?(Encoding) >> rv[path] >> Regexp.escape(path) >> diff --git a/test/test_linux.rb b/test/test_linux.rb >> index 65f25e0..953549e 100644 >> --- a/test/test_linux.rb >> +++ b/test/test_linux.rb >> @@ -1,6 +1,7 @@ >> # -*- encoding: binary -*- >> require 'test/unit' >> require 'tempfile' >> +require 'tmpdir' >> require 'raindrops' >> require 'socket' >> require 'pp' > > I noticed your second patch reverted this change. Better to use > "git commit --amend" to rewrite changes (especially trivial ones) > before sending them so you don't propagate the noise going forward. ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-06-05 15:46 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CALO9HP2oBDPBaeKx8N_Lp9GbfTOoGTFkKXcyfZJaJm=E_ksLOA@mail.gmail.com> 2012-06-05 0:07 ` [PATCH] Adds pathname support Brian Corrigan 2012-06-05 0:47 ` Eric Wong 2012-06-05 15:45 ` Brian Corrigan
Code repositories for project(s) associated with this public inbox https://yhbt.net/raindrops.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).