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.

Sign up for GitHub

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

Jump to bottom

createVisit/Project/Cohort imaging config #8384

Merged
merged 1 commit into from
Mar 6, 2023
Merged

createVisit/Project/Cohort imaging config #8384

merged 1 commit into from
Mar 6, 2023

Conversation

laemtl
Copy link
Contributor

@laemtl laemtl commented Feb 18, 2023 •
edited
Loading

New imaging config to set:

  • createVisit
  • a default project used if createVisit or createCandidate is set to true, or for phantom scans
  • a default cohort used if createVisit is set to true

@laemtl laemtl requested a review from cmadjar February 18, 2023 02:24
ridz1208
ridz1208 previously requested changes Feb 18, 2023
View reviewed changes
Copy link
Collaborator

@ridz1208 ridz1208 left a comment

Choose a reason for hiding this comment

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

Please replace all subproject references with cohort

@laemtl
Copy link
Contributor Author

laemtl commented Feb 18, 2023

Oh that's true :) Thanks!

@laemtl laemtl requested a review from ridz1208 February 18, 2023 03:08
@laemtl laemtl mentioned this pull request Feb 19, 2023
Merged
@laemtl laemtl added this to the 25.0.0 milestone Feb 28, 2023
cmadjar
cmadjar reviewed Mar 2, 2023
View reviewed changes
Copy link
Collaborator

@cmadjar cmadjar left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@laemtl @ridz1208 The new configs makes sense to me but was not able to test the configs on the MRI side as the code related to this PR on the MRI side has not yet been implemented. Will approve only once final testing on MRI side is complete.

cmadjar
cmadjar approved these changes Mar 6, 2023
View reviewed changes
Copy link
Collaborator

@cmadjar cmadjar left a comment

Choose a reason for hiding this comment

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

LGTM

@cmadjar cmadjar added the SQL PR contains SQL modifications such as schema changes and new SQL scripts label Mar 6, 2023
@cmadjar cmadjar changed the title createVisit/Project/Subproject imaging config createVisit/Project/Cohort imaging config Mar 6, 2023
@cmadjar cmadjar dismissed ridz1208’s stale review March 6, 2023 16:35

Subproject has been changed to Cohort in the PR.

@cmadjar cmadjar added the Blocking PR should be prioritized because it is blocking the progress of another task label Mar 6, 2023
@cmadjar
Copy link
Collaborator

cmadjar commented Mar 6, 2023

Added the Blocking label as this is blocking the merge of aces/Loris-MRI#809 on the MRI side.

@cmadjar
Copy link
Collaborator

cmadjar commented Mar 6, 2023

@driusan I believe this is ready for your final review. Thanks!

@driusan driusan merged commit f868cb4 into aces:main Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmadjar cmadjar cmadjar approved these changes

@ridz1208 ridz1208 Awaiting requested review from ridz1208

Assignees
No one assigned
Labels
Blocking PR should be prioritized because it is blocking the progress of another task SQL PR contains SQL modifications such as schema changes and new SQL scripts
Projects
None yet
Milestone
25.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@laemtl @cmadjar @ridz1208 @driusan