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

Rename field IsValidated in mri_upload table #1501

Conversation

MounaSafiHarab
Copy link
Contributor

to IsCandidateInfoValidated, and set its default to NULL
This is related to pull request: aces/Loris-MRI#106
in LORIS-MRI

…o IsCandidateInfoValidated in mri_upload table
@codecov-io
Copy link

Current coverage is 24.65%

Merging #1501 into 16.04-dev will increase coverage by +0.73% as of 181708b

@@            16.04-dev   #1501   diff @@
=========================================
  Files             110     110       
  Stmts           18444   18446     +2
  Branches            0       0       
  Methods          1024    1024       
=========================================
+ Hit              4413    4547   +134
  Partial             0       0       
+ Missed          14031   13899   -132

Review entire Coverage Diff as of 181708b

Powered by Codecov. Updated on successful CI builds.

@MounaSafiHarab MounaSafiHarab added this to the 16.04 milestone Jan 25, 2016
@driusan
Copy link
Collaborator

driusan commented Jan 25, 2016

Why the change in default from false to null? I can't think of any reasons this flag would need to be null and false seems like a reasonable default..

@MounaSafiHarab
Copy link
Contributor Author

To discern between the 3 cases (This is in reference to redmine task#7678):

  1. Preprocessing is run from the imaging_upload_file.pl step, which then gives two outputs: 1 if IsValidated is true and 0 if false. This validation only takes place if preprocessing happens using imaging_upload_file.pl (which does not take place if we run the preprocessing using option 2) below).
  2. Preprocessing is run from the tarchiveLoader step, in which case, the IsValidated should be set to default, or NULL (as opposed to 0).

@driusan
Copy link
Collaborator

driusan commented Jan 25, 2016

Is the reasoning is that there's 3 values, I'm not sure that having "null" be treated as a special magic value is the right way to go, because you need to memorize the difference between false and null. Maybe it should be a non-null enum (CandidateInfoValidityStatus?) with a default of something like 'Unknown'?

ted-strauss-K1 added a commit that referenced this pull request Feb 5, 2016
…ploadTable

 Rename field IsValidated in mri_upload table
@ted-strauss-K1 ted-strauss-K1 merged commit 695c548 into aces:16.04-dev Feb 5, 2016
@MounaSafiHarab MounaSafiHarab added Add to Release Notes PR change should be highlighted in Release notes (important security, features and bugfixes) Caveat for Existing Projects PR contains changes that might impact the code or accepted practices of current active projects SQL PR contains SQL modifications such as schema changes and new SQL scripts labels Mar 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Add to Release Notes PR change should be highlighted in Release notes (important security, features and bugfixes) Caveat for Existing Projects PR contains changes that might impact the code or accepted practices of current active projects SQL PR contains SQL modifications such as schema changes and new SQL scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants