[clang] [clang-format] Support of TableGen formatting. (PR #76059)

Björn Schäpers via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 28 03:11:33 PST 2023


HazardyKnusperkeks wrote:

> @rymiel @HazardyKnusperkeks Thank you for your review! I have fixed the points. But for refactoring of the test base class in [f8d10d5](https://github.com/llvm/llvm-project/commit/f8d10d5ac9ab4b45b388c74357fc82fb96562e66) . I'm not sure I should do here, and if I should, I should do it in splitted pull request.
> 
> Now I really understand I should split this pull request into some parts. At first it is large and continue growing by adding documents. I'm wondering how and current plan is separating semantically,
> 
>     * Handling multi line string (~100 lines).
> 
>     * Handling numeric like identifier (~100 lines).
> 
>     * Handling TableGen specific keywords (~100 lines)
> 
>     * Unwrapped line parsing(~100 lines).
> 
>     * Parse TableGen values (about 500+ lines including unittest).
> 
>     * Basic options (but for aligning ones) (about 500+ lines including the document).
> 
>     * Aligning options (about 100 lines including document).
> 
>     * Refactor unittests.
> 
> 
> I'm not sure this is good plan. They may be complicated. Could you help me to plan if you have some idea?
> 
> In addition, I do not know the appropriate way to split pull request after I made one. Is it enough to refer each other, and abort this at last?

Just create a pull request per feature/bug. You can sill use this for one of them, or abandon this. If some of them build on each other there is, as far as I know, no good way. 

The few times I came to this I created multiple PRs and the top most commit(s) is/are part of multiple PRs and I mentioned this. Then we have to make sure that they are merged correctly. With multiple local branches you can still work on all of them in parallel.

My experience (here and at work) is, that smaller changes are easier to check and you get a faster response. Because 50~150 lines changes I can just review and move on. A 500+ change review will more likely be postponed.

https://github.com/llvm/llvm-project/pull/76059


More information about the cfe-commits mailing list