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

docs: add warning about using absolute URLs. #63

Conversation

switowski
Copy link
Member

Signed-off-by: Sebastian Witowski witowski.sebastian@gmail.com

@switowski switowski changed the title docs: add warning about using absolute URLs. WIP docs: add warning about using absolute URLs. Oct 12, 2017
@switowski switowski force-pushed the 2017-10-12-do-not-use-jsonschema-urls-doc-improvement branch 2 times, most recently from e71b1a0 to e46a95a Compare October 12, 2017 11:56
@switowski switowski changed the title WIP docs: add warning about using absolute URLs. docs: add warning about using absolute URLs. Oct 12, 2017
Copy link

@ntarocco ntarocco left a comment

Choose a reason for hiding this comment

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

It looks good! I would maybe add specifically that it is not also recommended to "expose" the URLs, or is it clear enough?

Like:

, it is not recommended to store and expose the JSON schema URLs...
or other kind of identifiers with the certitude that they will never change, to avoid broken references.

Sorry for my poor english...

@switowski switowski force-pushed the 2017-10-12-do-not-use-jsonschema-urls-doc-improvement branch from e46a95a to 749ebaf Compare October 12, 2017 14:55
@switowski
Copy link
Member Author

Sounds good, I have updated the text!
@lnielsen do you think the usage chapter is a good place for this warning?

docs/usage.rst Outdated
@@ -1,6 +1,6 @@
..
This file is part of Invenio.
Copyright (C) 2015 CERN.
Copyright (C) 2015, 2016, 2017 CERN.
Copy link
Member

@drjova drjova Oct 13, 2017

Choose a reason for hiding this comment

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

why? I think we haven't touch it :)

@@ -22,7 +22,17 @@
# waive the privileges and immunities granted to it by virtue of its status
# as an Intergovernmental Organization or submit itself to any jurisdiction.

"""Invenio module for building and serving JSONSchemas."""
"""Invenio module for building and serving JSONSchemas.
Copy link
Member

Choose a reason for hiding this comment

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

2017? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I moved the text but forgot about the license :P

* Adds a note to the documentation, that absolute URLs in $ref should be
  avoided (closes inveniosoftware#23).

Signed-off-by: Sebastian Witowski <witowski.sebastian@gmail.com>
@switowski switowski force-pushed the 2017-10-12-do-not-use-jsonschema-urls-doc-improvement branch from 749ebaf to cd71cf9 Compare October 13, 2017 11:13
@switowski
Copy link
Member Author

It's ready to be merged
Let's waffle it!

@drjova drjova merged commit 0b6d79b into inveniosoftware:master Oct 13, 2017
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.

docs: add a Warning recommending not to store or advertise invenio-jsonschema URLs
3 participants