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

Add workflow to identify RPB ZSS in ALMA #42

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

TobiasNx
Copy link
Contributor

No description provided.

@acka47
Copy link
Contributor

acka47 commented Aug 24, 2023

This looks very good. The JSON can easily be imported in OpenRefine:

image

Apparently, the lobid-resources reconciliation service is down at the moment so I could not try out reconciliation.

Copy link
Contributor

@acka47 acka47 left a comment

Choose a reason for hiding this comment

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

+1

@acka47 acka47 requested a review from fsteeg August 31, 2023 12:52
build.sbt Outdated
@@ -17,7 +17,7 @@ libraryDependencies ++= Seq(
"org.metafacture" % "metafacture-flux" % "5.7.0-rc1",
"org.metafacture" % "metafacture-triples" % "5.7.0-rc1",
"org.metafacture" % "metafacture-formatting" % "5.7.0-rc1",
"org.metafacture" % "metafix" % "0.6.0-rc3",
"org.metafacture" % "metafix" % "0.6.0-SNAPSHOT",
Copy link
Member

Choose a reason for hiding this comment

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

This causes the build to fail.

@TobiasNx Did you need something specific in the SNAPSHOT, or was this just to make it work for you locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, it was just to make it work locally. The new release should be good.

Copy link
Member

Choose a reason for hiding this comment

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

Then I'll revert to 0.6.0-rc3 for now, which is configured in the GitHub CI action.

@TobiasNx TobiasNx assigned fsteeg and unassigned TobiasNx Sep 1, 2023
@fsteeg
Copy link
Member

fsteeg commented Sep 1, 2023

Assigning @acka47, since you have been working on the reconciliation in OpenRefine. Now that the build passes you can merge this if that makes sense to you (not quite sure about the state of this since it's a draft).

@fsteeg fsteeg assigned acka47 and unassigned fsteeg Sep 1, 2023
Copy link
Contributor Author

@TobiasNx TobiasNx left a comment

Choose a reason for hiding this comment

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

@acka47 the fix does not exclude DE-36 anymore.

@TobiasNx TobiasNx marked this pull request as ready for review September 20, 2023 12:12
@TobiasNx
Copy link
Contributor Author

@fsteeg should I merge this, or do you do it?

@fsteeg
Copy link
Member

fsteeg commented Oct 19, 2023

I think this should contain some documentation about what this is (there is no issue linked and no description provided here), maybe in the README? In particular where the input file comes from and how we are using the output.

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