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

From: Оля Тележная <olyatelezhnaya@gmail.com>
Date: Sep 29, 2017, 16:08 UTC
Message-ID: <CAL21BmkcVSEhEK+tAE-RNVabb0pnokYwbagueUrp9giZ3zqT8A@mail.gmail.com>
In-Reply-To: <CAP8UFD1-9dYSX-VKZSPN9Ei75V8mGC-wusieL45ArxxJ08tO9Q@mail.gmail.com>

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 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).

I've read only about 50 characters max in commit head (and highlighting repeats it), but there's nothing about max length of line in commit message. Sorry, next time I will make it shorter.

About many different opinions how to improve the code: I agree with the idea that my commit is a middle step to get rid of MRU at all. If we really need to add initializer/mru_for_each/smth_else - it's absolutely not a problem, but as it was said, not sure that we need it. It really looks that using list implementation from list.h directly won't be worse.

> I had envisioned leaving mru_mark() as a wrapper for "move to the front"
> that could operate on any list. But seeing how Olga's patch takes it
> down to two trivial lines, I'd also be fine with an endgame that just
> eliminates it.

Let's add needed function to list.h directly? I also wanted to add list_for_each_entry function to list.h as it's in Linux kernel. https://www.kernel.org/doc/htmldocs/kernel-api/API-list-for-each-entry.html It will simplify the code even more, guess that not only in MRU related code. Maybe we need to do that in separate patch.

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 have plans to rewrite (with --amend) my current commit (I think so because I will add no new features, so it's better to have single commit for all changes). As I understand, Submitgit will send an update in a new thread. And I need to say there [PATCH v2]. Please correct me if I am wrong in any of the moments mentioned earlier.

By the way, other contributors write smth like "[PATCH v6 0/3]". What does mean "0/3"? It's about editing separate commits in a single patch, am I right?

Thank you one more time! Olga