Re: [PATCH Outreachy] mru: use double-linked list from list.h

From: Jeff King <peff@peff.net>
Date: Sep 29, 2017, 23:40 UTC
Message-ID: <20170929234002.3fzaksoarz75p7e2@sigill.intra.peff.net>
In-Reply-To: <CAL21Bmma8gOYx9u4kxRaHJKcF3YsfrQP9=wdAiQX14f9uSPRAQ@mail.gmail.com>

On Fri, Sep 29, 2017 at 11:38:27PM +0300, Оля Тележная wrote:

Show 8 quoted lines

> > About minor issues ( "tmp" vs "p2", variable scope, space indentation)
> > - fully agree, I will fix it.
> > ...
> > So finally I think that I need to fix that minor issues and that's
> > all.
> 
> I forgot about leak. I also need to add checking in mru_clear. That's
> not beautiful solution but it works reliably.

I'm not sure what you mean about checking in mru_clear(). It may make sense to just send your v2 patch and I can see there what you do.

> Question remains the same - do you see something more to fix in this patch?

I think that's it. Think about writing a bit in the commit message about the "why" of this. Like I said earlier, the rationale for this commit is perhaps obvious, but it never hurts to be explicit (and it may be good practice, since part of the point of this task is getting you familiar with the patch submission process).

-Peff