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

[REF] Change the linked examples to recipe repo #158

Merged
merged 8 commits into from
Feb 3, 2024
Merged

[REF] Change the linked examples to recipe repo #158

merged 8 commits into from
Feb 3, 2024

Conversation

surchs
Copy link
Contributor

@surchs surchs commented Jan 29, 2024

Checklist

Copy link

netlify bot commented Jan 29, 2024

Deploy Preview for neurobagel-documentation ready!

Name Link
🔨 Latest commit 95a77d0
🔍 Latest deploy log https://app.netlify.com/sites/neurobagel-documentation/deploys/65be71d9803a940008a25eef
😎 Deploy Preview https://deploy-preview-158--neurobagel-documentation.netlify.app/nipoppy/configs
📱 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.

@alyssadai alyssadai self-requested a review January 31, 2024 17:07
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.

Hey @surchs, sorry for the delay with this review!

The changes are looking good, but I noticed there are still a few remaining sections on this docs page that refer to files from the neurobagel/api repo (I did a quick search for "api repo" to double check), which should also be updated as part of this PR.

Since I can't seem to comment directly on lines that weren't changed, I've summarized these in my last comment below!

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 Outdated Show resolved Hide resolved
docs/infrastructure.md Show resolved Hide resolved
surchs and others added 3 commits February 2, 2024 13:45
And also introduce a nice merge conflict because someone forget to merge
first
@surchs surchs requested a review from alyssadai February 2, 2024 20:21
@surchs
Copy link
Contributor Author

surchs commented Feb 2, 2024

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 the changes @surchs! Some final corrections below with regards to repo naming, and I think we're gtg 🧑‍🍳

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 Outdated Show resolved Hide resolved
docs/infrastructure.md Outdated Show resolved Hide resolved
docs/infrastructure.md Outdated Show resolved Hide resolved
@surchs
Copy link
Contributor Author

surchs commented Feb 3, 2024

The dead links check failing is expected, because these links are anticipating that neurobagel/recipes#15 will be merged as is.

@surchs surchs merged commit a107d1a into main Feb 3, 2024
5 of 6 checks passed
@surchs surchs deleted the issue144 branch February 3, 2024 17:06
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.

Switch local node setup instructions to use recipes repo
2 participants