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

Complement CA processor #17

Merged
merged 7 commits into from
Mar 11, 2024
Merged

Complement CA processor #17

merged 7 commits into from
Mar 11, 2024

Conversation

julianrojas87
Copy link
Collaborator

@julianrojas87 julianrojas87 commented Mar 9, 2024 •
edited
Loading

Changes include:

  • Use n3 DataFactory for SDS blank node generation. The one from rdf-data-factory crashes for some reason.
  • Implement logic for supporting --before and --after date filters.
  • Add support for multiple shape descriptions
  • Add tests for the Connector Architecture processor.
  • Update dependencies.
  • Comment typos.

@julianrojas87
Fix gitignore
1426d17
@julianrojas87
Update dependencies
c86eaf2
@julianrojas87
Add connector architecture tests
a334125
@julianrojas87
Changes include:
b3ee3e6 
- Use n3 DataFactory for SDS blank node generation. The one from rdf-data-factory crashes.
- Implement logic for supporting before and after date filters.
- Add tests for Connector Architecture processor.
- Update dependencies.
- Comment typos.
@julianrojas87 julianrojas87 changed the title Ca processor Complement CA processor Mar 9, 2024
@pietercolpaert
Copy link
Member

pietercolpaert commented Mar 11, 2024 •
edited
Loading

I don’t think this can be merged:

  • Using N3 data factory all of a sudden is not a great idea: identifiers might start conflicting
  • --before and --after date filters → Please provide tests for that... EDIT: tests are there: https://github.com/TREEcg/ldes-client/blob/ca-processor/tests/connector-architecture/ldes-client.test.ts
  • Add support for multiple shape descriptions: multiple shapes need to be added in the sh:NodeShape itself through sh:or. Otherwise it’s unclear whether to sh:and (the default in RDF) or sh:or is to be produced. I think this should be removed from the PR.

ajuvercr
ajuvercr approved these changes Mar 11, 2024
View reviewed changes
Copy link
Member

@ajuvercr ajuvercr left a comment

Choose a reason for hiding this comment

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

The logic to determine which relations to follow could be cleaner, but it's fine

@ajuvercr ajuvercr merged commit 7ccd41f into main Mar 11, 2024
1 check passed
@ajuvercr ajuvercr deleted the ca-processor branch March 11, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajuvercr ajuvercr ajuvercr approved these changes

Assignees

@ajuvercr ajuvercr

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@julianrojas87 @pietercolpaert @ajuvercr