Re: [gentoo-dev] [PATCH] fortran-2.eclass: support EAPI 7

[Andrew Savchenko]

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