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

Revert "Merge pull request #97 from qurator-spk/420-namespace-package" #108

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

bertsky
Copy link
Contributor

@bertsky bertsky commented Jun 10, 2023

This reverts commit fd56b86, reversing changes made to ea792d1.

This is the second attempt, same reasoning as in #107 (creating an upstream ref for ocrd_all), but different target: We want to avoid #86 (entirely), but also #97. We cannot just revert to v0.2.0 though, because we do need #100. So this is #100 plus a revert of #97.

…ace-package"

This reverts commit fd56b86, reversing
changes made to ea792d1.
@mikegerber
Copy link
Member

This is already merged it seems.

@cneud
Copy link
Member

cneud commented Aug 17, 2023

Yes, the changes introduced by this PR so far are all in main already - but also the PR is still marked as draft.

@bertsky what do you consider the status of this?

@bertsky
Copy link
Contributor Author

bertsky commented Aug 18, 2023

Oh my... confusion on all levels!

This is already merged it seems.

No, it's not. You can clearly see a diff here.

Also, it's not even meant to be merged (only provide a reference for ocrd_all as long as problems caused by #86 and #97 are not fixed on main), hence the draft status. See reasoning in #107.

Yes, the changes introduced by this PR so far are all in main already - but also the PR is still marked as draft.

@bertsky what do you consider the status of this?

Again: #86 is broken for OCR-D. So is #97. But we still do need #100 (which obviously came later). So this is effectively a state before #86 plus cherry-picked #100.

@cneud
Copy link
Member

cneud commented Aug 18, 2023

@bertsky I see...sorry for the confusion and thanks for clarifying (again).

@bertsky
Copy link
Contributor Author

bertsky commented Jan 19, 2024

Unfortunately, I'll have to add new changes to this PR, which will likely be needed on master as well: Since OCR-D/core#1079, we must correctly distinguish between local_filename and url.

If all else fails, please consider just cherry-picking from here on.

@bertsky
Copy link
Contributor Author

bertsky commented Jan 24, 2024

Moreover, since @kba says local_filename will now officially be : Path, we need yet another fix here.

@bertsky bertsky mentioned this pull request May 27, 2024
@kba
Copy link
Contributor

kba commented Aug 24, 2024

I think I have cherry-picked all the commits to #130

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.

4 participants