Skip to content

Fix for jena-iri IRI.isAbsolute#2061

Merged
afs merged 3 commits intoapache:jena5from
afs:jena5-abs-iri
Oct 26, 2023
Merged

Fix for jena-iri IRI.isAbsolute#2061
afs merged 3 commits intoapache:jena5from
afs:jena5-abs-iri

Conversation

@afs
Copy link
Copy Markdown
Member

@afs afs commented Oct 26, 2023

GitHub issue resolved #2060

This closes #2060

Various fixes in in ARP show that the change works. Several "isAbsolute" needed to be "!isRelative".


By submitting this pull request, I acknowledge that I am making a contribution to the Apache Software Foundation under the terms and conditions of the Contributor's Agreement.

@afs afs changed the title Foix for jena-iri IRI.isAbsolute Fix for jena-iri IRI.isAbsolute Oct 26, 2023
Copy link
Copy Markdown
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

The code looks good, thanks for the cleanup and comments. Would a unit test be useful (or possible) here too?

@afs
Copy link
Copy Markdown
Member Author

afs commented Oct 26, 2023

Tests added.

Largely, I'd coded around the problem by using "isRelative" and "! isRelative" but with tests the change is pushed down in jena-iri.

I found this while comparing jena-iri with iri4ld and there'll be comparison tests for different IRIPRovider implementations some time. This is all about being very, very careful in exact expected behaviour of IRIs.

Copy link
Copy Markdown
Member

@kinow kinow left a comment

Choose a reason for hiding this comment

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

🚀

@afs afs merged commit c41f162 into apache:jena5 Oct 26, 2023
@afs afs deleted the jena5-abs-iri branch October 26, 2023 14:34
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.

2 participants