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

[SQL] Setting default value of ScannerID in mri_protocol to be NULL instead of 0 #7496

Merged
merged 3 commits into from
Jul 7, 2021
Merged

[SQL] Setting default value of ScannerID in mri_protocol to be NULL instead of 0 #7496

merged 3 commits into from
Jul 7, 2021

Conversation

vietdhoang
Copy link
Contributor

@vietdhoang vietdhoang commented Jun 29, 2021 •
edited
Loading

This PR sets the default value of ScannerID in the mri_protocol to be NULL. Before these changes, the default value was 0. This PR addresses Issue #7476 and should not be merged unless #640 is ready to be merged as well.

Testing instructions

  1. Run the following query to check which entries have ScannerID=0.
SELECT * FROM mri_protocol WHERE ScannerID=0;
  1. Source the file 2021-06-23_set_default_ScannerID_to_NULL.sql
source SQL/New_patches/2021-06-23_set_default_ScannerID_to_NULL.sql
  1. Check that ScannerID is NULL for the entries that had ScannerID=0 from step 1.
  2. Rerun RaisinBread installation process and check that it works.

@vietdhoang vietdhoang added Category: Bug PR or issue that aims to report or fix a bug Language: SQL PR or issue that update SQL code labels Jun 29, 2021
@vietdhoang vietdhoang mentioned this pull request Jun 29, 2021
Merged
@vietdhoang vietdhoang added the State: Needs work PR awaiting additional work by the author to proceed label Jun 29, 2021
cmadjar
cmadjar requested changes Jun 30, 2021
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.

@vietdhoang Looks good to me. A few minor changes in the text of the CHANGELOG.md file.

CHANGELOG.md Outdated Show resolved Hide resolved
@vietdhoang @cmadjar
Update CHANGELOG.md
342220d 
Co-authored-by: Cécile Madjar <cecile.madjar@mcin.ca>
cmadjar
cmadjar approved these changes Jul 2, 2021
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 Passed manual tests PR has been successfully tested by at least one peer Blocking PR should be prioritized because it is blocking the progress of another task and removed State: Needs work PR awaiting additional work by the author to proceed labels Jul 2, 2021
@cmadjar
Copy link
Collaborator

cmadjar commented Jul 2, 2021

This is blocking the merge of the following PR on the LORIS-MRI side: aces/Loris-MRI#640

@cmadjar
Copy link
Collaborator

cmadjar commented Jul 2, 2021

@driusan ready for you :)

@driusan driusan merged commit ecbd09b into aces:main Jul 7, 2021
@ridz1208 ridz1208 modified the milestone: 24.0.0 Aug 24, 2021
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

Assignees
No one assigned
Labels
Blocking PR should be prioritized because it is blocking the progress of another task Category: Bug PR or issue that aims to report or fix a bug Language: SQL PR or issue that update SQL code Passed manual tests PR has been successfully tested by at least one peer
Projects
None yet
Milestone
24.0.0
Development

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