All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Lachlan McIlroy <lachlan@sgi.com>
Cc: xfs@oss.sgi.com, markgw@sgi.com
Subject: Re: TAKE 981498 - Use xfs_idestroy() to cleanup an inode.
Date: Wed, 20 Aug 2008 21:15:03 +1000	[thread overview]
Message-ID: <20080820111503.GC6061@disturbed> (raw)
In-Reply-To: <48ABB602.5030404@sgi.com>

On Wed, Aug 20, 2008 at 04:13:22PM +1000, Lachlan McIlroy wrote:
> This change was part of another patch that you reviewed.  This small
> change got left out when I merged my changes in with your inode
> allocation cleanup which you asked me to do. 

Yes, I did ask for you to do that and I kinda expected to see the
result for review again after that. I did not review the changes
that were committed.

A second review would have caught the bug you introduced by
integrating the bug fix into my patch as I would have suggested that
you keep the enhancment and the bug fix as two separare commits.
Then the commit logs that would have a 'use init_once' commit and a
'deadlock + memory leak fix' commit....

> I also had to modify
> your original patch because it did not apply cleanly due other changes
> that you made (the semaphore completion stuff).

You could have asked for an updated patch when you found it
didn't apply. I had one ready to go and ended up posting
it twice before your commit...

> I didn't have to take
> your cleanup patch - I could have just fixed the bug.

Your choice, but either way I kind of expect some kind of dialogue
when changes are neceessary.  It only takes a few seconds to send a
'doesn't apply - can you update/going with original bugfix'
message.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

      reply	other threads:[~2008-08-20 11:14 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-20  3:33 TAKE 981498 - Use xfs_idestroy() to cleanup an inode Lachlan McIlroy
2008-08-20  3:56 ` Dave Chinner
2008-08-20  6:13   ` Lachlan McIlroy
2008-08-20 11:15     ` Dave Chinner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080820111503.GC6061@disturbed \
    --to=david@fromorbit.com \
    --cc=lachlan@sgi.com \
    --cc=markgw@sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.