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

Openownership work needed #8

Closed
wants to merge 8 commits into from
Closed

Openownership work needed #8

wants to merge 8 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 25, 2018

There are several things happening here:

  1. Remove the code that disabled get_remote_json. Now I'm not entirely sure why this was disabled in the first place - maybe just enabling it again will cause other issues?

  2. Pass base_uri to jsonref.load - this is needed or it can't load relative files.

  3. Remove Python 2 support. If we really want this, then I've added pathlib.Path which is 3 only.

Discussed with @kindly on call but now put as PR for all to discuss.

This is related to openownership/data-standard-sphinx-theme#4

@Bjwebb
Copy link
Member

Bjwebb commented Jul 9, 2018

Remove the code that disabled get_remote_json. Now I'm not entirely sure why this was disabled in the first place - maybe just enabling it again will cause other issues?

Yes this causes issues. For open contracting we have a URI for each of the schema files, which sometimes doesn't yet have a schema at it (e.g. after we increment the URI, but after we have a build to copy to the relevant locations) - here's an example https://github.com/open-contracting/standard/compare/example-1__1__4-bump
I'm aware that maybe isn't very clear, so happy to have a quick call.

@ghost
Copy link
Author

ghost commented Dec 2, 2020

Closing. This still needs to be looked at, but must be done in a different way.

@ghost ghost closed this Dec 2, 2020
@ghost ghost deleted the openownership branch December 2, 2020 13:56
@kindly kindly restored the openownership branch February 25, 2021 22:32
This pull request was closed.
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