tlc-pack / relax

Apache License 2.0
193 stars 58 forks source link

[Discuss] Merge `main` into `relax` instead of doing rebase #371

Open kparzysz-quic opened 1 year ago

kparzysz-quic commented 1 year ago

There are several issues with doing rebases:

  1. Rebase rewrites all commits in the rebased branch.
  2. Rebases cannot be done incrementally (they actually grow with time).

We're planning to integrate Relax into our ongoing development, which would entail frequently including new "relax" commits (without waiting for the "official" relax-TVM synchronization. If the "relax" branch continues to be rebased on top of "main", the point (1) above would make it somewhat painful for us to stay up-to-date with it.

We have a similar situation with our downstream development branch and the upstream "main". What we do is that we merge (git merge) the upstream "main" branch into our downstream branch on an essentially daily basis. The benefits of doing that are:

  1. All commit SHAs are preserved: git pull works before and after a merge.
  2. Merges can be done incrementally (if there is only 1 new commit in "main", the merge will bring only 1 new commit). Likelihood of conflicts decreases significantly.

We'd like to propose that similar process is done for synchronizing the "relax" branch with the "main" branch. Downstream consumers (like us) could merge both "main" and "relax" into their development branches, and these merges would coexist seamlessly.

TejashShah commented 1 year ago

cc @YuchenJin @jwfromm

slyubomirsky commented 1 year ago

It's worth considering how much we value having a linear commit history. One possibility (if we really love having a linear commit history) could be to use merging while this repo is a separate fork and later rewrite history only once when (I hope) Relax is merged into mainline TVM, rather than doing rebases every so often.

slyubomirsky commented 1 year ago

Points from the Jan. 24 Relax open developer meeting:

We did not come to a conclusion at the meeting and will continue to discuss this subject.

driazati commented 1 year ago

I missed the meeting but I believe merging makes a lot of sense here, especially since it works out of the box without us having to build anything to hack on top of git or add any non-standard tooling (e.g. the script described above). With the octoml/relax in the picture as well we would be rebasing on top of rebases which makes github a little upset.

slyubomirsky commented 1 year ago

@driazati Continued discussion of this issue is on the agenda for tomorrow's discussion; you're welcome to attend

slyubomirsky commented 1 year ago

Notes from the continued discussion of this issue in the Jan. 31, 2023, open development meeting:

Meeting consensus: Strong majority in favor of trying an experiment. We will set up a fork and try merging from main, report back on any issues, and discuss whether we should do it in the unity branch after the initial establishment

supersat commented 1 year ago

If we tag all relax commits with [Relax], could we filter the commits to get a simple relax-only linear timeline?

kparzysz-quic commented 1 year ago

If we tag all relax commits with [Relax], could we filter the commits to get a simple relax-only linear timeline?

If we maintain linear order within the relax/unity branch, then yes.