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.

Sign up for GitHub

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

Jump to bottom

Feature/577 complete onix 3 1 #664

Open
wants to merge 22 commits into
base: develop
Choose a base branch
from
Open

Feature/577 complete onix 3 1 #664

wants to merge 22 commits into from
+4,062 −3

Conversation

brendan-oconnell
Copy link
Contributor

@brendan-oconnell brendan-oconnell commented Jan 10, 2025 •
edited
Loading

@brendan-oconnell
First commit, update changelog
4b91be5
@brendan-oconnell
Added first draft of ONIX 3.1 specification, copying from ONIX 3.0 Thoth
6ef6697
@brendan-oconnell
Updated description to reflect release 3.1.2
052f2e3
@brendan-oconnell
Changed all references from 3.1 to 3.1.2
ea0b01d
@brendan-oconnell
Implement AffiliationIdentifier block and update tests
eac4d8e
@brendan-oconnell
Fixed indentation for TextItem, PageRun, and NumberOfPages to include…
8593886 
… inside ContentItem
@brendan-oconnell
Merge with develop
6906977
@brendan-oconnell
Added chapter license, title, and contributions to work export. Contr…
51f52ff 
…ibutions not fully working yet
@brendan-oconnell
Add biography and website to relatedWork
05a3c16
@brendan-oconnell
Add languages to WorkRelationsRelatedWork and update relevant tests
92db670
@brendan-oconnell
Add tests for Contributor and Language
ec6492b
@brendan-oconnell
Add abstract and copyright holder to chapter
5622122
@brendan-oconnell
Merge branch 'develop' into feature/577_complete_onix_3_1
4a493dc 
Merge with develop
@brendan-oconnell
Add references
b786769
@brendan-oconnell
Refactor license for chapter and work
52a0712
@brendan-oconnell
Refactor title, subtitle for work and chapter
3032efc
@brendan-oconnell
Add copyright, short abstract
863572e
@brendan-oconnell
Refactor long abstract, general note
660ec97
@brendan-oconnell
Fix formatter and clippy errors
201b447
@brendan-oconnell brendan-oconnell self-assigned this Feb 3, 2025
@brendan-oconnell brendan-oconnell linked an issue Feb 3, 2025 that may be closed by this pull request
Produce complete ONIX 3.1 #577
Open
@brendan-oconnell
Refactor open access statement for chapter and work, correct indentat…
bc03a1d 
…ion for <PageRun> and <NumberOfPages> and move inside <TextItem> composite, break tests into smaller sections
@brendan-oconnell
Fix formatter errors
a16f0bc
@brendan-oconnell
Update changelog
e736177
@brendan-oconnell brendan-oconnell marked this pull request as ready for review February 4, 2025 14:45
ja573
ja573 requested changes Feb 11, 2025
View reviewed changes
thoth-export-server/src/data.rs Show resolved Hide resolved
thoth-export-server/src/xml/onix312_thoth.rs Show resolved Hide resolved
thoth-export-server/src/xml/onix312_thoth.rs Show resolved Hide resolved
thoth-export-server/src/xml/onix312_thoth.rs Show resolved Hide resolved
thoth-export-server/src/xml/onix312_thoth.rs
for language in &chapter.languages {
XmlElementBlock::<Onix312Thoth>::xml_element(language, w).ok();
}
if chapter.short_abstract.is_some()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split these into individual ifs, removing the extra check inside the function (you're checking if either of these attributes is some, and then again in each function)

thoth-export-server/src/xml/onix312_thoth.rs
.map_err(|e| e.into())
})?;
let identifiers: Vec<(&str, Option<&str>)> = vec![
("programname", self.program.as_deref().to_owned()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no need to clone with to_owned since Option<&str> is just a reference

thoth-export-server/src/xml/onix312_thoth.rs
write_element_block("FundingIdentifier", w, |w| {
// 01 Proprietary
write_element_block("FundingIDType", w, |w| {
w.write(XmlEvent::Characters("01")).map_err(|e| e.into())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All .map_err(|e| e.into()) can be turned into .map_err(Into::into)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ja573 ja573 ja573 requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@brendan-oconnell brendan-oconnell

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Produce complete ONIX 3.1
2 participants
@brendan-oconnell @ja573