[clang] [llvm] [WebAssembly] Implement an alternative translation for -wasm-enable-sjlj (PR #84137)

YAMAMOTO Takashi via cfe-commits cfe-commits at lists.llvm.org
Mon Mar 25 16:53:55 PDT 2024


yamt wrote:

> I'm not sure about the common practice in this repo, but Github in general supports merge workflows much better. Especially in a long-running large PRs, if you force-push, it's hard for the reviewers to know what has changed since their last review, because all the history his lost and they have to look at the whole diff again. And many PRs in this repo tend to be bigger and long-running than other repos in my opinion.
> 
> Also Github supports a nice feature called "New changed since you last reviewed" ![image](https://private-user-images.githubusercontent.com/8726997/315571735-72b62f34-86a3-4195-9a17-9a8276d2a886.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MTE0MTA5NDksIm5iZiI6MTcxMTQxMDY0OSwicGF0aCI6Ii84NzI2OTk3LzMxNTU3MTczNS03MmI2MmYzNC04NmEzLTQxOTUtOWExNy05YTgyNzZkMmE4ODYucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MDMyNSUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDAzMjVUMjM1MDQ5WiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9Yzk2Y2RlMWY2OWIyYmQxZGIxNzFmN2Q3ZTQxNmVmNjFlMzVjNjcwOTdjYzZkMTA4MzBiYjUyZWMyMGVlMjU1YiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QmYWN0b3JfaWQ9MCZrZXlfaWQ9MCZyZXBvX2lkPTAifQ.p5p_VER8neYefnI6cpw_1ydj9lUGvuONN3JjWrBByjE) and this feature is usable only with the merge workflow. (The screenshot is not mine; I got it from Google) I wonder why the merge workflow is harder to review.
> 
> Also when you merge a PR, you do "squash and merge", so all those merge commits are not gonna end up in the main branch because they are squashed into a single commit, so you don't need to worry about that. (If you do "Create a merge commit" the story is different but that option is not enabled for this repo and I don't think it's used often elsewhere either)

Ok. Maybe it's fine for projects like this, where we squash commits when landing.


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


More information about the cfe-commits mailing list