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

feat(github): add documentation #163

Merged
merged 6 commits into from
May 15, 2024
Merged

feat(github): add documentation #163

merged 6 commits into from
May 15, 2024

Conversation

vit-zikmund
Copy link
Contributor

Well, here's the final drop to make my humble GitHub contribution complete.
Please, @rufuspollock @athornton have a look at it, the English text will surely contain some weird Czechisms 🇨🇿 and stuff 🤣.

The docs reflect the state after PR #162

#### `LEGACY_ENDPOINTS`
This is a `bool` flag, default `true` (deprecated, use `false` where possible), that affects the base URI of all the service endpoints. Previously, the endpoints didn't adhere to the rules for [automatic LFS server discovery](https://github.com/git-lfs/git-lfs/blob/main/docs/api/server-discovery.md), which needed additional routing or client configuration.

The default base URI for all giftless endpoints is now `/<org_path>/<repo>.git/info/lfs` while the legacy one is `/<org>/<repo>`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool. It may take me personally some time to convert all our repos, but having discovery work is awesome.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With this fast Giftless release pace... I'm really worried you might not make it in time! 😄


services:
giftless:
image: docker.io/datopian/giftless:latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

We might actually want to point to the lsst-sqre one for now. I haven't been making any headway on getting access to PyPi and Docker from Datopian. And honestly I don't want write access to their repositories, but I would like to put newer versions out there.

Copy link
Contributor Author

@vit-zikmund vit-zikmund May 14, 2024

Choose a reason for hiding this comment

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

Well, that's a [Datopian] shame, but in this example it's just a local name (as the pull policy is cleverly set to Never 🤣) The image is always being built locally (only using that one as the cache source - and that itself is of questionable benefit at the moment - it's there to reflect exactly how the Makefile does it)

Copy link
Collaborator

@athornton athornton left a comment

Choose a reason for hiding this comment

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

This is fantastic!

There are a couple places where the English could be more idiomatic. I've opened a ticket for myself and hopefully I get that touched up this week. If I don't, we should merge it Friday May 17 anyway, because it's far better than it was.

Not sure what to do about the default image, in that I am maintaining one, but it's not the official Datopian one.

@vit-zikmund
Copy link
Contributor Author

Thank you very much once again! Don't worry about the images, no matter how awesome is that you're maintaining a separate one. I'm explaining that in the related thread.

@rufuspollock
Copy link
Member

Greta work everyone and especially @vit-zikmund here. Thank you and looking forward to seeing this merged.

@athornton
Copy link
Collaborator

Eh, I'm going to go ahead and merge, and hopefully I circle back and do some touchups, and if I don't, well, it's still a massive improvement.

@athornton athornton merged commit a984aa0 into datopian:main May 15, 2024
7 checks passed
@vit-zikmund vit-zikmund deleted the update-docs branch May 16, 2024 08:26
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.

3 participants