qiyao wrote:

> LGTM! Please be careful when merging: the PR message is outdated (the commit 
> message seems correct). When you merge, have a look at what GitHub shows as 
> the preview for the final message: will it use the PR description, or the 
> commit message?
> 

Thanks for reminding me.  I'll double check the message is my commit message.

> One small note about the llvm convention here: while I personally prefer the 
> squash+force push approach, this is not the convention in the LLVM repo. 
> Here, the prescribed approach is to make a change and then "git commit 
> --fixup", so that reviewers can inspect how feedback was addressed. (Github 
> is configured to squash everything upon merging)

Good to know the  "git commit --fixup" tip.  I usually add delta commits to 
address review comments (TIL `--fixup`),  design of this PR was changed after 
the review each time, so I take the force push approach.

https://github.com/llvm/llvm-project/pull/208347
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to