-
Notifications
You must be signed in to change notification settings - Fork 78
fix: Expand URN acronym #792
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
Conversation
ee9f13a to
f247e16
Compare
f247e16 to
2b47826
Compare
wgresshoff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm currently overhauling this myself (because the module was chnaged) but still your changes are good input! Thanks for your input!
| - Authentication: "operate/customize/authentication.md" | ||
| - DOI registration: "operate/customize/dois.md" | ||
| - DNB URNs registration: "operate/customize/urns.md" | ||
| - URN registration: "operate/customize/urns.md" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since per @wgresshoff comment this only works for Deutsche National Bibliothek (DNB) URN, we are better off keeping it there:
| - URN registration: "operate/customize/urns.md" | |
| - DNB URN registration: "operate/customize/urns.md" |
Taking a step back, more context in docs/operate/customize/urns.md would help. We should spell out "Deutsche National Bibliothek (DNB)" and make it clear that this is for those URNs only. (We have lots of German institutions using InvenioRDM, so it's reasonable right now).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me, DNB URN registration is too specific for the sidebar and draws too much attention, but we can put it if you disagree. Agree with spelling out DNB though :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's indeed just for DNB URNs , I think having it on the sidebar is the right thing to do 😁 (better be explicit, and shorter titles catch my attention more than longer ones haha!).
In any case there is another PR for this now: #805 (review) So maybe @wgresshoff will adapt things there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wgresshoff could you include my changes here in your PR? (besides from the removal of DNB in mkdocs.yml)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed, will push soon :) As I have completely removed the OAI xMetaDiss part from the module (there are common custom fields now, so I don't care for the ones I defined any more) that part was not ported 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, I hope I got it right now 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfekt :)
|
Closing in favour of #805 |
❤️ Thank you for your contribution!