Opened 5 years ago
Closed 5 years ago
#1825 closed enhancement (fixed)
PATCH: various improvements to MODS XML export requested by eXist solutions
| Reported by: | karnesky | Owned by: | ajlyon |
|---|---|---|---|
| Priority: | minor | Milestone: | |
| Component: | translators | Version: | 2.1 |
| Keywords: | Cc: | ajlyon, simon |
Attachments (2)
Change History (9)
comment:1 Changed 5 years ago by karnesky
- Cc ajlyon added
- Component changed from uncategorized to translators
- Owner changed from dstillman to ajlyon
- Priority changed from major to minor
comment:2 Changed 5 years ago by karnesky
- Version changed from 1.0 to 2.1
comment:3 Changed 5 years ago by ajlyon
- Cc simon added
- Status changed from new to accepted
comment:4 Changed 5 years ago by karnesky
eXist will do additional testing. I suspect the patch is fine, but would suggest getting their go-ahead before committing (and this should come fairly soon).
If it were just me, I'd leave out the issuance information: I don't think it adds anything (a client that used the MODS we generate would be able to make the same reasonable guess about the issuance as we do without us providing this & providing it implies a bit of certainty). But I don't feel strongly. If it helps eXist & doesn't seem to hurt anyone else, we should probably leave it in.
comment:5 Changed 5 years ago by karnesky
Just a quick note that eXist is happy with the patch as-is.
comment:6 Changed 5 years ago by ajlyon
I'm having general XML import issues with the Zotero trunk that I need to address before checking this in, so I can properly test it and add test cases. Once we address #1854, I'll make this land. Sorry for the continuing delays, and thanks to Rick and eXist for your hard work.
comment:7 Changed 5 years ago by ajlyon
- Resolution set to fixed
- Status changed from accepted to closed
Committed in: https://github.com/zotero/translators/commit/370f6581d87c9b4319af1f52e2dd1a52e39f453b, additional changes and test cases in: https://github.com/zotero/translators/commit/f11b74aa694494544c5579a777fbe0558879f9c0
It'd be great if eXist or someone tested this out some more, and perhaps even crafted some test cases using Scaffold (and a trunk or branch development XPI of Zotero). It works fine-- I'm just not 100% confident that all the desired changes made it through intact.
Further discussion can be directed to zotero-dev, or reopen the ticket if this substantially failed to address the requirements.
In general, this looks good and clean. As I've said, I don't know MODS well enough to comment on the behavior changes, so I'll defer to the judgment of the submitters in general and karnesky in particular.
Is there any downside to having the issuance information that eXist has pointed out? I see that it may not be 100% accurate, but the assumption looks reasonable.
Once we make a decision there, I'm happy to commit this.