[pacman-dev] [PATCH 0/2] Remove REQUIREDBY usage

[Dan McGee]

On Nov 13, 2007 2:01 PM, Nagy Gabor <ngaba at bibl.u-szeged.hu> wrote:
> > On Nov 13, 2007 12:47 PM, Nagy Gabor <ngaba at bibl.u-szeged.hu> wrote:
> > > Any notable optimization worth the hack.

I disagree, just as Aaron stated below. The biggest reason? This is
code that crosses the libalpm/pacman barrier. When that happens,
interfaces should be as clean as possible.

> > I just need to comment here that I'd prefer we _not_ do this in
> > pacman. "Any optimization is worth it" is a REALLY bad mantra when it
> > comes to code, in my experience. Code should be clean and elegant
> > _first_ and as optimized as possible _second_. So if optimizations
> > break code clarity and add unnecessary stuff for a 0.1 second
> > improvement, it's not worth it.
> >
> > Sure large improvements are worth a loss in code clarity, but, as Dan
> > actually proved with the timings, there's very little room for
> > improvement here.
>
> But you should see, that your reasoning is simply impossible to keep,
> because "clean code" and "elegance" are _subjetive_ things:
Sure they are. And unfortunately since Aaron and I are the committers
to the projects.al.org GIT tree, you will be held to our definitions
of clean code and elegance. And I'm not trying to sound nasty here-
I'm just laying out the facts.

> -_imho_ sync.c is not elegant at all
I don't believe we have ever disagreed with you here, so no need to
fan the flames on this. Aaron and I didn't write this code, we've
inherited it. At the same time, it works, and a monster patch that I
can't digest isn't the solution to fixing it unless there is a great
deal of discussion on how the new code works.

> -_imho_ your alpm_list patch (first prev == tail) is not elegant (I would have
> preferred alpm_list_add_first)
A few issues here.
1. This is completely inside libalpm- there is no noticeable interface
change to the "end user" of libalpm.
2. This is a huge performance increase. Thus, we can sacrifice some
elegance for what we did. And it was commented well.
3. alpm_list_add_first would not help when it comes to printing the
filelists of all the packages, etc. It would print them all in reverse
order, which would confuse users.

> -_imho_ a function which we could _radically_ speed-up with a 'first'
> parameter and an 'if(first) return ...' line, is not elegant
Hmm?

Please keep this constructive if you (anyone on the list) respond to
this thread. We all want to work on pacman, hear bitching about what
"elegant" means.

-Dan