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

Create additional fields for hbz publishing catalogue #1

Merged
merged 55 commits into from
Sep 3, 2024

Conversation

TobiasNx
Copy link

@TobiasNx TobiasNx commented Aug 16, 2024

Related to #6. Create tags for specific hbz catalogue.

I am not sure:

  • How to reuse the GND Elements without copying them.

@TobiasNx
Copy link
Author

I am not sure how we can reuse the DNB tags without copying them?

@TobiasNx TobiasNx force-pushed the createAdditionalFieldsForHbzPublishingCatalogue branch from 1f5674f to 8edc7ac Compare August 19, 2024 10:15
@TobiasNx TobiasNx marked this pull request as ready for review August 19, 2024 10:35
@TobiasNx TobiasNx requested a review from Phu2 August 19, 2024 10:35
@Phu2
Copy link

Phu2 commented Aug 20, 2024

I'm not sure either if 517869a#diff-6ab233441d2457fd4d4370036d454637fc9ac8133b4d0226ffffa3b206391d18 will work...

We should resolve the fest failures first, then try it out. What do you think?

@Phu2
Copy link

Phu2 commented Aug 20, 2024

@TobiasNx
Copy link
Author

I'm not sure either if 517869a#diff-6ab233441d2457fd4d4370036d454637fc9ac8133b4d0226ffffa3b206391d18 will work...

We should resolve the fest failures first, then try it out. What do you think?

Sounds good!

@Phu2
Copy link

Phu2 commented Aug 20, 2024

I think we still have to follow this step

c. modify the expected number of data elements at src/test/java/de/gwdg/metadataqa/marc/utils/DataElementsStaticticsTest.java

in the README but don't know how to identify this number. @TobiasNx Any clue?

@Phu2
Copy link

Phu2 commented Aug 20, 2024

Ah build log says:

294.1 Failed tests: testStatistics(de.gwdg.metadataqa.marc.utils.DataElementsStaticticsTest): expected:<228> but was:<262>

@Phu2
Copy link

Phu2 commented Aug 20, 2024

Next test failure:

Failed tests: testPatternMatchingWithSubfield(de.gwdg.metadataqa.marc.analysis.bl.UseCaseTest): expected:<[110$e, 111$e, 100$e, 700$e, 710$e, 711$e, 720$e, 751$e, 752$e, 775$e, 788$e]> but was:<[100$e, 110$e, 111$e, 700$e, 710$e, 711$e, 720$e, 751$e, 752$e, 775$e, 788$e]>

https://github.com/hbz/qa-catalogue/actions/runs/10472986275/job/29003791025#step:4:16625

@TobiasNx 100$e is expected at 3rd position, but it's at 1st ??

@TobiasNx
Copy link
Author

Next test failure:

Failed tests: testPatternMatchingWithSubfield(de.gwdg.metadataqa.marc.analysis.bl.UseCaseTest): expected:<[110$e, 111$e, 100$e, 700$e, 710$e, 711$e, 720$e, 751$e, 752$e, 775$e, 788$e]> but was:<[100$e, 110$e, 111$e, 700$e, 710$e, 711$e, 720$e, 751$e, 752$e, 775$e, 788$e]>

hbz/qa-catalogue/actions/runs/10472986275/job/29003791025#step:4:16625

@TobiasNx 100$e is expected at 3rd position, but it's at 1st ??

I am confused, I think I did not change anything that touches this test. But I will have another look.

@@ -0,0 +1,91 @@
package de.gwdg.metadataqa.marc.definition.tags.hbztags;

import org.apache.hadoop.shaded.org.jline.builtins.telnet.Telnet;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess it is a mistake.

@TobiasNx
Copy link
Author

TobiasNx commented Sep 2, 2024

I added the DNB elements that are re-used in hbz again until pkiraly#507 is solved.

@TobiasNx
Copy link
Author

TobiasNx commented Sep 3, 2024

@Phu2 should we merge?

@Phu2
Copy link

Phu2 commented Sep 3, 2024

Commits 55, Files changed 110. Wow ;) Let's merge.

@TobiasNx TobiasNx merged commit 607a8b2 into main Sep 3, 2024
2 of 3 checks passed
@TobiasNx TobiasNx deleted the createAdditionalFieldsForHbzPublishingCatalogue branch September 3, 2024 10:13
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.

3 participants