search

UB-Mannheim / zotero-ocr

Zotero Plugin for OCR
GNU Affero General Public License v3.0
552 stars 40 forks source link

Zotero 7-only plugin, submitted for review #59

Closed aborel closed 8 months ago

aborel commented 8 months ago

This PR is in response to issue https://github.com/UB-Mannheim/zotero-ocr/issues/52 Functionality is essentially the same as for the master branch, but supporting Zotero 7 instead of Zotero 6 and older. I included a minor change that might also address https://github.com/UB-Mannheim/zotero-ocr/issues/46

aborel commented 8 months ago

My bad, the PR was a bit hasty - I have messed up something while preparing it. I'll report again when it's fixed.

aborel commented 8 months ago

False alarm, the submitted code is actually OK (as far as I can tell). Feedback is not only welcome, but needed 😅

aborel commented 8 months ago

Thanks for the review! I'll take a closer look in a few days.

mpr1255 commented 8 months ago

Really looking forward to giving this a spin!!

stweil commented 8 months ago

Can you squash the commits which remove .DS_Store files (a3ad7cbdf8933c9dde931a628391c6e78ddd51d1, ee11b268b7ae651924d405ec5374cb46c6bb978b) with their predecessors to clean the history a bit?

stweil commented 8 months ago

@aborel, would it be okay if I squash your commits with all suggested changes (perhaps with a subject line "Add support for Zotero 7"), add updates to support Zotero 6 and 7 simultaneously and then create a new pull request for that?

aborel commented 8 months ago

@aborel, would it be okay if I squash your commits with all suggested changes (perhaps with a subject line "Add support for Zotero 7"), add updates to support Zotero 6 and 7 simultaneously and then create a new pull request for that?

Yes, no problem!

stweil commented 8 months ago

@aborel, would it be okay if I squash your commits with all suggested changes (perhaps with a subject line "Add support for Zotero 7"), add updates to support Zotero 6 and 7 simultaneously and then create a new pull request for that?

Yes, no problem!

Thanks. See pull request #63 for the result. It now also builds an xpi file which can be used for testing automatically (see actions).

stweil commented 8 months ago

Meanwhile the changes from this pull request were committed to the master branch in commit 2f8e4ca9e7499de6d17ab22764c37f7bb4959102, so they are part of the new release 0.7.0 which now supports Zotero 7, too.

Thanks a lot for this great contribution!

zuphilip commented 8 months ago

Superseded by #63