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

137 use iris in hqdm rdf entity factories #156

Merged
merged 11 commits into from
Jan 18, 2024

Conversation

twalmsley
Copy link
Collaborator

Comment copied from the corresponding issue:

So, if I changed only the Rdf*Services classes then that's only half the job - the classes in the hqdm module would still need conversion to and from IRIs in the rest of the code, so I decided to change them as well to reduce clutter further. In summary:

  1. The hqdm-rdf module has been subsumed into the hqdm module. (I could have gone the other direction, but this seemed to be the right way to go).
  2. Any use of the RDF*Services is now replaced by the *Services in the hqdm module.
  3. There is no need to convert IRI objects to and from String objects when using the classes, and no need to create temporary IRI objects any more.

I have tested the impact on client code and it is a breaking change, but it makes the MagmaCore library easier to use. The two projects I tried it on both have quite a few changes to make but the resulting code is less cluttered and less clunky.

@twalmsley twalmsley linked an issue Jan 10, 2024 that may be closed by this pull request
@twalmsley twalmsley mentioned this pull request Jan 10, 2024
Copy link
Collaborator Author

@twalmsley twalmsley left a comment

Choose a reason for hiding this comment

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

Looks like I can't approve this since it's my PR - looks good though so feel free to merge when you're ready 👍

@GCHQDeveloper42
Copy link
Contributor

The hqdm-rdf-canonical module should probably now be renamed to hqdm-canonical.

Copy link
Collaborator Author

@twalmsley twalmsley left a comment

Choose a reason for hiding this comment

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

👍 Approved

@GCHQDeveloper42 GCHQDeveloper42 merged commit 168fc0a into main Jan 18, 2024
3 checks passed
@GCHQDeveloper42 GCHQDeveloper42 deleted the 137-use-iris-in-hqdm-rdf-entity-factories branch January 18, 2024 16:44
Copy link
Contributor

@GCHQDeveloper56 GCHQDeveloper56 left a comment

Choose a reason for hiding this comment

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

Well worth doing this as it results in far cleaner code and, importantly, the interfacing to services. Looks good to me!

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.

Use IRIs in hqdm-rdf entity factories
3 participants