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

Thank you for the amazing action! #1

Open
10 tasks
berezovskyi opened this issue Oct 16, 2024 · 0 comments
Open
10 tasks

Thank you for the amazing action! #1

berezovskyi opened this issue Oct 16, 2024 · 0 comments

Comments

@berezovskyi
Copy link

berezovskyi commented Oct 16, 2024

@elytvyno @pheyvaer thanks for making this action!

I just set it up on my repo to convert some random JSON files into minimal W3C SPARQL 1.2 Service Description dumps. I used R2RML ca. 10 years ago and using your tool and YARRRML is a breeze!

I have some feedback, I will paste it below but if you have interest to take it further, I can split it up into one issue per item (and, cautiously, maybe can submit PRs - never developed GH actions before, though):

  • support for dir separators in the glob - GLOBAL_PATTERN: "blabla/*.yml" fails because find does not like it (‘-name’ matches against basenames only, but the given pattern contains a directory separator (‘/’), thus the expression will evaluate to false all the time. Did you mean ‘-wholename’?)
  • avoid committing state into the repo (rml_action_meta) - would be good to put it behind a flag. But, perhaps, you wanted to avoid spurious changes by RDF resource reordering - https://www.w3.org/TR/rdf-canon/ could be helpful.
  • add a flag for pretty-printed Turtle - mainly blank nodes but not critical; may be related to https://www.w3.org/TR/rdf-canon/
  • Node 16 deprecation address - switch to Node 20; see https://github.blog/changelog/2023-09-22-github-actions-transitioning-from-node-16-to-node-20/
  • Update README with latest checkout@v2 action declaration
  • ignore empty string - example (I think this is due to bad JSON where the key was present with the "" value - would be good to have some processor to ignore that and treat such props as missing)
  • try git pull --rebase before push - otherwise the action run and all re-runs fail if a new push was made to the repo
  • do not run npm i, it can change installed deps between executions; add a lockfile and run npm ci - though I am not an expert in JS
  • configurable suffix instead of _output including no suffix
  • the action is a wee bit behind the RMLio/rmlmapper-java releases

Again, this GH Action and YARRRML are seriously good! Thank you so much.

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

No branches or pull requests

1 participant