Hello. I’m working on tests for CWG issues resolved in P1787, and I’m not sure what’s the best way to submit patches due to sheer amount of issues it resolves (70+). One patch per issue? One patch per group of issues, e.g. “Lookup completeness”? Single patch for the whole paper?
Thanks for working on that!
It’s also not clear to me how to tackle this paper in case it changes things not tracked by a specific issue.
To your question, if it’s just test code, i think multiple issues per patch is fine, but if there need to be change in the compiler, then 1 patch per change would be preferable
Thank you. I’m not getting into fixing those issues in compiler, at least just yet. I tried it once with CWG1960, but got overwhelmed by complexity of name lookup code.
There is no reasonable way to approach this paper. We ran into the same thing with one of the C DRs that I affectionately labelled “39 unrelated questions about C89”. The changes resolve core language issues:
In addition, issues
325 361 1089 1635
are partially resolved.
and we test DRs by adding test cases and special comment markup to files in llvm-project/clang/test/CXX/drs at main · llvm/llvm-project · GitHub. My recommendation is to add tests to the appropriate files within this directory. If the individual tests are small, I think it’s fine to group them into a single patch for review. However, if any of the tests are larger or more complicated (basically, something expected to require some discussion), then I’d split those out into their own patch so we can still land the easy stuff quickly.
Would it be too much pressure on reviewers if I submit one patch per issue by default? I got through two dozens of them already, and I believe the only trivial ones are pure wording changes that doesn’t affect implementation.
Maybe I treat those issues more seriously than they deserve, but it feels that once an entry on DR status page becomes green, barely anybody seriously look at test code.
If the reviews all come in at the same time, that’s probably going to add a fair amount of review pressure for folks (there’s > 60 issues in total and review throughput usually slows down this time of year due to the holidays), but if they’re spaced out a bit, I don’t think it’s a problem. So I’d say start with one issue per patch and be flexible if the reviewers ask for something different for subsequent patches.