Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2.70.0 backport to v3 #22

Open
wants to merge 139 commits into
base: new-processor-api
Choose a base branch
from

Conversation

kba
Copy link
Collaborator

@kba kba commented Oct 17, 2024

I'm currently rolling out v3 in our test cluster and just realized that I had not merged v2.70 with all the METS-Server-in-Processing-Server fixes. This PR rectifies that.

If okay with you, I would like to create a new b6 from this.

stweil and others added 30 commits September 12, 2024 18:45
@bertsky
Copy link
Owner

bertsky commented Oct 17, 2024

@kba it seems wrong that there are so many old commits here. This would reintroduce my v2 backports into v3, creating a highly confusing history AFAICS. Shouldn't the recent v2 changes rather be rebased onto new-processor-api prior to creating a PR?

@kba
Copy link
Collaborator Author

kba commented Oct 17, 2024

I (naively) just merged master into the v3 branch.

I can rebase only v2.69.0..v2.70.0 into new-processor-api. But won't that break mergeability with master? I agree that backporting backports is confusing but we will need to do it eventually to have a consistent git commit history for master.

@kba
Copy link
Collaborator Author

kba commented Oct 17, 2024

I (naively) just merged master into the v3 branch.

I can rebase only v2.69.0..v2.70.0 into new-processor-api. But won't that break mergeability with master? I agree that backporting backports is confusing but we will need to do it eventually to have a consistent git commit history for master.

Sorry, I am confused. Do you mean rebasing new-processor-api onto v2 master? I can do that, but that will require force pushing and probably invalidating a lot of references, won't it?

@kba
Copy link
Collaborator Author

kba commented Oct 17, 2024

I'm fairly certain this was a waste of time but I once I was a few commits in I did finish it: https:/bertsky/core/tree/v2.70.0-backport-to-v3-rebase new-processing-api rebased onto master.

@bertsky
Copy link
Owner

bertsky commented Oct 17, 2024

I can rebase only v2.69.0..v2.70.0 into new-processor-api.

Sounds good.

Or just the recently merged PR branches.

Or (in the extreme), just cherry-pick a list of commits to rebuild the PR branch.

But won't that break mergeability with master?

new-processor-api is currently (i.e. already) blocked because of the backports. So there will have to be some manual resolving involved, anyway.

I doubt that it will get more difficult this way (rather then via plain merge).

I agree that backporting backports is confusing but we will need to do it eventually to have a consistent git commit history for master.

The eventual merge will be in the opposite direction. It will v3's history to master. So we should try to keep v3 history consistent. Backporting backports should not be part of that process (and is unnecessary).

Sorry, I am confused. Do you mean rebasing new-processor-api onto v2 master?

I did not, actually. But that would probably help reducing history complexity.

I'm fairly certain this was a waste of time but I once I was a few commits in I did finish it: https:/bertsky/core/tree/v2.70.0-backport-to-v3-rebase new-processing-api rebased onto master.

Interesting. The summary says this is merely 164 commits ahead of master, instead of 221 commits.

If it serves your (intermediate) purpose, fine. But I doubt we should switch to that in OCR-D#1240. I have multiple branches based on new-processor-api which I would then have to rebase in turn. Also, we would loose the discussions and reviews in OCR-D#1240.

@MehmedGIT
Copy link
Collaborator

MehmedGIT commented Oct 17, 2024

I peeked over and identified the changes from my recent PRs. Overall seems good. I still have not verified if that works due to local issues I currently have with ocrd_all. I would suggest to go forward with the merges. If something with ocrd_network is broken, I could then fix that with small patches.

@kba
Copy link
Collaborator Author

kba commented Oct 17, 2024

I have tried a few different approaches and all are problematic:

  • interactive rebasing (hard to identify all the commits that belong to #1240 backports for v2 OCR-D/core#1275 and drop them)
  • manually selecting all the relevant non-merge commits and then doing individual cherry-picks (requires constant fixing of conflicts, which defeats the purpose)

So, in the interest of a clean git history (and my sanity), would it be okay if I did

git checkout new-processor-api
git checkout -p master . # manually, once for the whole codebase, separate the new stuff from the rewrites
git commit -m '!! backports !!'

Then at least, once we do finalize v3.0.0, we can rebase-drop that and do a proper merge but not have to deal with the branches, merges and duplicated commits right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants