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

On Sat, Sep 30, 2017 at 09:09:11PM +0300, Оля Тележная wrote: > I added "v2" after "PATCH", but it does not appeared. Actually it was > written automatically and it was "PATCH Outreachy v2". I rearranged it > in the middle of the phrase. That looks fine. > > I'm not sure what you mean about

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

I added "v2" after "PATCH", but it does not appeared. Actually it was written automatically and it was "PATCH Outreachy v2". I rearranged it in the middle of the phrase. >> I forgot about leak. I also need to add checking in mru_clear. That's >> not beautiful solution but it works reliably. > >

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

Jeff King writes: > Yes, I think we could just call this "list_move_to_front()" or > something. The fact that it's operating on a list called > "packed_git_mru" is probably sufficient to make it clear that the > purpose is managing recentness. I earlier said I wasn't sure, but I

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

On Fri, Sep 29, 2017 at 11:38:27PM +0300, Оля Тележная wrote: > > 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

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

On Fri, Sep 29, 2017 at 07:08:28PM +0300, Оля Тележная wrote: > Many thanks to all of you, I am interested in every opinion. Sorry > that I wasn't in the discussion, unfortunately I got sick, that's why > I skipped all the process. No problem. It's often reasonable to let review comments come in

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

> 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

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

Hi everyone, Many thanks to all of you, I am interested in every opinion. Sorry that I wasn't in the discussion, unfortunately I got sick, that's why I skipped all the process. I want to reply to the main moments and also ask some questions. >> Simplify mru.c, mru.h and related code by reusing

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

On Fri, Sep 29, 2017 at 9:23 AM, Jeff King wrote: > On Fri, Sep 29, 2017 at 09:18:11AM +0200, Christian Couder wrote: > >> As we use the "prepare_packed_git_run_once" static, this function will >> only be called only once when packed_git_mru has not yet been >> initialized, so

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

On Fri, Sep 29, 2017 at 09:18:11AM +0200, Christian Couder wrote: > On Fri, Sep 29, 2017 at 12:42 AM, Jeff King wrote: > > On Thu, Sep 28, 2017 at 08:38:39AM +, Olga Telezhnaya wrote: > > > >> diff --git a/packfile.c b/packfile.c > >> index f69a5c8d607af..ae3b0b2e9c09a 100644

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

On Fri, Sep 29, 2017 at 12:42 AM, Jeff King wrote: > On Thu, Sep 28, 2017 at 08:38:39AM +, Olga Telezhnaya wrote: > >> diff --git a/packfile.c b/packfile.c >> index f69a5c8d607af..ae3b0b2e9c09a 100644 >> --- a/packfile.c >> +++ b/packfile.c >> @@ -876,6 +876,7 @@ void

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

On Thu, Sep 28, 2017 at 08:38:39AM +, Olga Telezhnaya wrote: > diff --git a/packfile.c b/packfile.c > index f69a5c8d607af..ae3b0b2e9c09a 100644 > --- a/packfile.c > +++ b/packfile.c > @@ -876,6 +876,7 @@ void prepare_packed_git(void) > for (alt = alt_odb_list; alt; alt = alt->next) >

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

On Fri, Sep 29, 2017 at 06:56:28AM +0900, Junio C Hamano wrote: > Jeff King writes: > > > But I also think this patch may be a stepping stone to dropping "struct > > mru" entirely, and just pushing a "struct list_head mru" into the > > packed_git object itself (or of course any

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

Jeff King writes: > But I also think this patch may be a stepping stone to dropping "struct > mru" entirely, and just pushing a "struct list_head mru" into the > packed_git object itself (or of course any object you like). At which > point we'd just directly use the list iterators

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

On Thu, Sep 28, 2017 at 08:38:39AM +, Olga Telezhnaya wrote: > Simplify mru.c, mru.h and related code by reusing the double-linked > list implementation from list.h instead of a custom one. The commit message is a good reason to talk about why we want to do this. In this case, the answer may

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

On Thu, Sep 28, 2017 at 08:03:00PM +0900, Junio C Hamano wrote: > > - for (entry = packed_git_mru.head; entry; entry = entry->next) { > > + list_for_each(pos, _git_mru.list) { > > + struct mru *entry = list_entry(pos, struct mru, list); > > struct packed_git *p =

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

Olga Telezhnaya writes: > Simplify mru.c, mru.h and related code by reusing the double-linked list > implementation from list.h instead of a custom one. An overlong line (I can locally wrap it, so the patch does not have to be re-sent only to fix this alone). >

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

Simplify mru.c, mru.h and related code by reusing the double-linked list implementation from list.h instead of a custom one. Signed-off-by: Olga Telezhnaia Mentored-by: Christian Couder Mentored by: Jeff King ---