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

Broken stac-spec links #114

Closed
duckontheweb opened this issue Mar 22, 2021 · 10 comments
Closed

Broken stac-spec links #114

duckontheweb opened this issue Mar 22, 2021 · 10 comments
Milestone

Comments

@duckontheweb
Copy link
Contributor

Links to entities defined in the radiantearth/stac-spec repo are set up as relative links within the stac-spec folder. Since this is actually a submodule these links don't work as expected and should maybe be updated to be absolute URLs.

I can work on a PR to fix this if it's not already underway.

@cholmes
Copy link
Collaborator

cholmes commented Mar 22, 2021

So the problem with absolute links is keeping them up to date and properly pinned to the right STAC release. We don't want to just go to master since that may change. The submodule seemed to give us control and easy updates.

Is it not working even when you've checked the sub-module out correctly? Or are you just saying that people shouldn't have to check out the sub-module to have links work right?

@duckontheweb
Copy link
Contributor Author

The problem I'm seeing is with the README hosted on GitHub. Someone going to https://github.com/radiantearth/stac-api-spec and clicking on the link to one of those objects (e.g. the Catalog link in the "About" section of the main README) gets a 404 response.

@cholmes
Copy link
Collaborator

cholmes commented Mar 22, 2021

Fair point, and yeah, that seems less than ideal. I wonder if they used to work, as it seems like github has changed some of their links around recently.

A PR to fix would be great, and then perhaps also file a ticket to spur us to get a robust way to update the links when we release.

@duckontheweb
Copy link
Contributor Author

Actually, using a git subtree instead of a git submodule might solve our problem here. Subtrees actually copy the child repo into the parent, which would keep links intact (versus submodules, which just point to a commit on the child repo). It requires git>=v1.7, but that version is years old, so it shouldn't be a problem.

@duckontheweb
Copy link
Contributor Author

Actually, using a git subtree instead of a git submodule might solve our problem here. Subtrees actually copy the child repo into the parent, which would keep links intact (versus submodules, which just point to a commit on the child repo). It requires git>=v1.7, but that version is years old, so it shouldn't be a problem.

The advantages to this solution are that relative links would work and it continues with the pattern of embedding that spec within this one the same way that we currently do with git submodule. The disadvantages are that the entire stac-spec codebase would then also be duplicated here and would be editable. This means that we would probably want some system to ensure that PRs to this repo don't make changes to the stac-spec folder that would then put it out of sync with the actual repo.

I'm happy to put together a PR to see what this would look like if folks are interested, but using absolute links to the stac-spec might just be simpler.

@m-mohr @matthewhanson @cholmes Curious what you all think would be best.

@m-mohr
Copy link
Collaborator

m-mohr commented Apr 15, 2021

@duckontheweb I think I'd be in favor of subtree than absolute links, if we can make it that PRs to the subtree get rejected/can't be merged or so.

@cholmes
Copy link
Collaborator

cholmes commented Apr 16, 2021

+1 on the subtree. To me that's ok even if we can't get automatic rejection of the PR's, since we'll soon turn on the requirement of 2 reviewers as we have in stac-spec. Reviewers would know that edits to stac-spec aren't supposed to be done here.

@duckontheweb duckontheweb mentioned this issue Apr 20, 2021
3 tasks
@philvarner philvarner added this to the 1.0.0-beta.2 milestone May 5, 2021
@philvarner
Copy link
Collaborator

@duckontheweb did your PR resolve this issue?

@duckontheweb
Copy link
Contributor Author

@duckontheweb did your PR resolve this issue?

Yes, you can close this. I'll also close #115, which is now no longer necessary.

@philvarner
Copy link
Collaborator

Great, thanks!

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

No branches or pull requests

4 participants