public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
From: Andrew Savchenko <bircoph@gentoo.org>
To: gentoo-dev@lists.gentoo.org
Subject: Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7
Date: Mon, 5 Nov 2018 18:37:10 +0300	[thread overview]
Message-ID: <20181105183710.3aedce77c5d7601f39dd06ab@gentoo.org> (raw)
In-Reply-To: <1541168416.899.14.camel@gentoo.org>

[-- Attachment #1: Type: text/plain, Size: 3672 bytes --]

On Fri, 02 Nov 2018 15:20:16 +0100 Michał Górny wrote:
> 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.

EAPI update usually don't affect full ebuild scope. For EAPI 6->7
update grep is sufficient to find affected parts of the eclass.

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

Not all eclasses are frequently used, including fortran-2 eclass.

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

While peer review is important in many cases, the statement above
is wrong. The free software (and not just open source and Gentoo
is free software) founds on the four freedoms as defined by FSF.

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

Everything is good only within sane limits. While peer review is
often useful, it may be harmful as well:

1) It at least doubles human time required to do the job. And human
resources are the only resources we are really short of.

2) Sometimes it turns into bikeshedding, e.g. of how variables
should be named or what indentation style should be used.

3) Unreasonably long and complicated process for routine changes
may discourage people from further contributions. Right now we have
dozens of eclasses still on EAPI 6. I would prefer to see them
EAPI 7 so they would not block EAPI 7 updates of dependent packages,
rather than require them to underdo complete overhaul during EAPI 7
update effectively postponing it for a long time.

Sometimes the best is the enemy of the good.

Best regards,
Andrew Savchenko

[-- Attachment #2: Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-11-05 15:39 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
2018-11-05 15:37           ` Andrew Savchenko [this message]
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=20181105183710.3aedce77c5d7601f39dd06ab@gentoo.org \
    --to=bircoph@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