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

2034-newAdjestUrnRegex #2043

Merged
merged 6 commits into from
Aug 8, 2024
Merged

2034-newAdjestUrnRegex #2043

merged 6 commits into from
Aug 8, 2024

Conversation

TobiasNx
Copy link
Contributor

@TobiasNx TobiasNx commented Aug 7, 2024

See #2034
Replaces #2035

This PR tries to catch all URN resolver links not just nbn from 865.

It uses the URNs from 024 as fallback if no 865 links with URN are provided.
Also it changes the nbn links to org when creating them as fallback.

@TobiasNx TobiasNx requested a review from maipet August 7, 2024 08:18
@TobiasNx TobiasNx mentioned this pull request Aug 7, 2024
@TobiasNx TobiasNx assigned blackwinter and unassigned maipet Aug 7, 2024
@TobiasNx
Copy link
Contributor Author

TobiasNx commented Aug 7, 2024

@blackwinter since you became a reviewer by commenting, your approval is still pending.

@blackwinter
Copy link
Member

But it's not required, is it? I'm just offering suggestions. Feel free to deal with them however you see fit.

If you're asking for approval, though, I'm inclinded to suggest that you adjust the other regexp as well.

@TobiasNx
Copy link
Contributor Author

TobiasNx commented Aug 8, 2024

But it's not required, is it? I'm just offering suggestions. Feel free to deal with them however you see fit.

If you're asking for approval, though, I'm inclinded to suggest that you adjust the other regexp as well.

Feel free to suggest the change for the other one, always good to improve code.

Copy link
Member

@blackwinter blackwinter left a comment

Choose a reason for hiding this comment

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

If we wanted to be even more pedantic, we'd change all the capturing parentheses ((...)) to non-capturing ones ((?:...)).

src/main/resources/alma/fix/relatedRessourcesAndLinks.fix Outdated Show resolved Hide resolved
@blackwinter blackwinter assigned TobiasNx and unassigned blackwinter Aug 8, 2024
Co-authored-by: Jens Wille <[email protected]>
@TobiasNx TobiasNx merged commit 6374048 into master Aug 8, 2024
1 check passed
@TobiasNx TobiasNx deleted the 2034-newAdjestUrnRegex branch August 8, 2024 09:09
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.

4 participants