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

[ENH] Provide instructions for federation API #133

Merged
merged 19 commits into from
Nov 28, 2023
Merged

[ENH] Provide instructions for federation API #133

merged 19 commits into from
Nov 28, 2023

Conversation

surchs
Copy link
Contributor

@surchs surchs commented Nov 20, 2023

Changes proposed in this pull request:

Checklist

Copy link

netlify bot commented Nov 20, 2023

Deploy Preview for neurobagel-documentation ready!

Name Link
🔨 Latest commit d073c87
🔍 Latest deploy log https://app.netlify.com/sites/neurobagel-documentation/deploys/6565fab3a75203000800365c
😎 Deploy Preview https://deploy-preview-133--neurobagel-documentation.netlify.app/federate
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@alyssadai alyssadai left a comment

Choose a reason for hiding this comment

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

Thanks for all this great documentation @surchs!

The overall order and grouping of the new instructions make a lot of sense to me, and I do feel this separation of the query tool from the main API-graph stack makes things a little easier to understand.

I've added suggestions below which are mostly related to clarity and minor config fixes (it looks like GitHub is hiding a bunch of them under the "hidden conversations"), along with a few questions that may warrant some new issues to address.

Have a look and see what makes sense to you.

A general comment: it looks like our inline capitalization of "Neurobagel" varies a lot across our docs. Maybe worth converging on one standard capitalization (either "neurobagel" or "Neurobagel")?

docs/federate.md Outdated Show resolved Hide resolved
docs/federate.md Outdated Show resolved Hide resolved
docs/federate.md Show resolved Hide resolved
docs/federate.md Outdated Show resolved Hide resolved
docs/federate.md Show resolved Hide resolved
docs/infrastructure.md Outdated Show resolved Hide resolved
docs/infrastructure.md Outdated Show resolved Hide resolved
docs/infrastructure.md Outdated Show resolved Hide resolved
docs/infrastructure.md Show resolved Hide resolved
docs/infrastructure.md Show resolved Hide resolved
docs/federate.md Outdated Show resolved Hide resolved
docs/federate.md Outdated Show resolved Hide resolved
Copy link
Contributor

@alyssadai alyssadai left a comment

Choose a reason for hiding this comment

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

One 🍒, otherwise good to go! 🎉 Thanks again for all your work on this @surchs!

Co-authored-by: Alyssa Dai <[email protected]>
@surchs surchs merged commit c9edf14 into main Nov 28, 2023
6 checks passed
@surchs surchs deleted the issue129 branch November 28, 2023 14:36
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.

document how to run F-API locally
2 participants