From: "Michał Górny" <mgorny@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Subject: Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7
Date: Fri, 02 Nov 2018 15:20:16 +0100 [thread overview]
Message-ID: <1541168416.899.14.camel@gentoo.org> (raw)
In-Reply-To: <20181102012744.475046f4a4cf9aadf5f17c67@gentoo.org>
[-- Attachment #1: Type: text/plain, Size: 2962 bytes --]
On Fri, 2018-11-02 at 01:27 +0300, Andrew Savchenko wrote:
> Hi!
>
> On Tue, 30 Oct 2018 08:18:58 +0100 Michał Górny wrote:
> > On Mon, 2018-10-29 at 03:57 +0300, Andrew Savchenko wrote:
> > > On Sun, 28 Oct 2018 19:29:28 +0100 Michał Górny wrote:
> > > > On Sun, 2018-10-28 at 01:38 +0300, Andrew Savchenko wrote:
> > > > > Hi all!
> > > > >
> > > > > The only blocker for EAPI 7 update is eutils inheritance, but it
> > > > > seems to be not used within the current eclass code, probably a
> > > > > remnant from older days. So it is removed.
> > > > >
> > > > > Looks like no other EAPI 7 specific changes needed.
> > > > >
> > > >
> > > > Please use -U99999 to include more context to the patches.
> >
> > I'm going to include a few 'easy cleanup' comments since EAPI 7
> > is a good opportunity to improve the eclass. I'm going to skip horribly
> > bad design decisions since I suppose nobody cares.
>
> Should we really mix EAPI bump with full code review?
Yes, for two reasons.
Firstly, because an EAPI bump effectively requires reviewing all
the eclass logic for constraints imposed by the new EAPI. While
reviewing code, it is natural that people may notice other issues.
Ignoring them once noticed would be a waste of effort.
Secondly, changes to frequently used eclass have a large overhead of
metadata cache updates. Given most of the listed issues are rather
trivial to fix, it would be wasteful to defer them for a second metadata
cache update.
> This eclass is small, so no harm here. But for larger eclasses
> (hello java-*.eclass) this will hinder updates considerably. I
> prefer to fix something rather than to fix nothing while
> frustrating in attempt to fix everything at once.
>
> Also this make git history review harder as fixes for independent
> issues will be mixed together.
Why would you mix them together? The whole point of using git (and not
CVS) is that you can trivially make separate commits addressing
different kinds of issues. It also makes it trivial to send them for
review afterwards.
> So I kindly ask you for future updates (from everyone, not just
> me) focus on review of the proposed changes instead of reviewing
> full code. Thank you for understanding.
As explained above, the proposed change is meaningless without context
(as it affects how everything else in eclass works). If we were to
ignore context, we'd even ACK eclass changes that resulted in the eclass
immediately dying due to programmer's mistake.
Finally, I'd like to point out that peer review is one of foundations
of open source. Sadly, Gentoo has failed to embrace this, and right now reviews of existing code are rather an exception than a rule. What
makes it even worse is that some developers are actively hostile to
the criticism of their code. It is as if Gentoo's bazaar was dominated by makeshift cathedrals.
--
Best regards,
Michał Górny
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 963 bytes --]
next prev parent reply other threads:[~2018-11-02 14:20 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-27 22:38 [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7 Andrew Savchenko
2018-10-28 18:29 ` Michał Górny
2018-10-29 0:57 ` Andrew Savchenko
2018-10-30 7:18 ` Michał Górny
2018-11-01 22:27 ` Andrew Savchenko
2018-11-02 0:47 ` Michael Orlitzky
2018-11-02 14:20 ` Michał Górny [this message]
2018-11-05 15:37 ` Andrew Savchenko
2018-10-29 22:52 ` Andreas K. Huettel
2018-11-01 22:26 ` Andrew Savchenko
2018-11-01 22:25 ` [gentoo-dev] [PATCH v2] " Andrew Savchenko
2018-11-02 10:27 ` Ulrich Mueller
2018-11-05 14:30 ` Andrew Savchenko
2018-11-05 15:37 ` [gentoo-dev] [PATCH v3 2/2] " Andrew Savchenko
2018-11-17 11:38 ` Andrew Savchenko
2018-11-05 15:38 ` [gentoo-dev] [PATCH v3 1/2] " Andrew Savchenko
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=1541168416.899.14.camel@gentoo.org \
--to=mgorny@gentoo.org \
--cc=gentoo-dev@lists.gentoo.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox