[flang-commits] [PATCH] D82695: [flang] Roll up work on external I/O runtime library

[Peter Klausler via Phabricator via flang-commits]

klausler added a comment.

In D82695#2125841 <https://reviews.llvm.org/D82695#2125841>, @richard.barton.arm wrote:

> In D82695#2120042 <https://reviews.llvm.org/D82695#2120042>, @klausler wrote:
>
> > In D82695#2119300 <https://reviews.llvm.org/D82695#2119300>, @richard.barton.arm wrote:
> >
> > > Please can this patch be split up into separate patches for each of the separate types changes? Roll-up patches are not good practice and against the developer policy <https://llvm.org/docs/DeveloperPolicy.html#incremental-development>. 
> > >  @PeteSteinfeld The patches should also be code reviewed for LLVM master. Prior review on some other project is not relevant and should not be given as justification for approving patches to LLVM master.
> > >  The clang-tidy linting errors also need fixing up.
> > >
> > > Thanks
> >
> >
> > I'm happy to not push this code if you don't want it.
>
>
> That is not the situation at all. I do very much want the code.
>
> What I am asking for is for the code to be submitted in incremental patches, with each patch addressing a single concern rather than as one big patch. Submitting code in this way helps the quality of the codebase in a number of ways: it makes the changes easier to review; if a particular subset of the whole change causes a regression then it can easily be reverted by itself rather than requiring the whole change to be reverted; the commit messages can be better and it generally makes the commit history easier to understand in the future if single commits do single things - particularly valuable when inspecting regression test provenance.
>
> I would gladly review the code in smaller increments. A good starting point would be one increment per bullet point in your summary.


I wasted a few days trying to disentangle all of this interlocked work on the external I/O runtime into smaller chunks, but it's a big reworking with many iterations of many provisional function implementations and some data structures.  Carving it up isn't hard; carving it up into incremental patches that each continue to build successfully is difficult to do in a way that doesn't look completely artificial.  I have many other things to do at the moment, but maybe I'll take another attempt at repackaging this runtime work later this month.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82695/new/

https://reviews.llvm.org/D82695