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

[Richard Barton via Phabricator via flang-commits]

richard.barton.arm added a comment.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82695