[Everything] DIE MRI ALIAS DIE #1227
Conversation
| =cut | ||
|
|
||
|
|
||
| sub getCenterNameFromCenterID { |
There was a problem hiding this comment.
Personal opinion: I would prefer this function to be named getSiteMRIAliasFromSiteID.
- "center" is used pervasively in the LORIS code but Samir said in some LORIS / LORIS-MRI meeting that "site" is the preferred terminology. Since it does against the current code I guess this is somewhat controversial.
- A "site name" refers to another column in the database, so I think it would be less confusing to specify "MRIAlias" directly.
There was a problem hiding this comment.
I'll avoid the entire situation and change it to getMRIAliasFromCenterID()
There was a problem hiding this comment.
Is there a reason why we are getting rid of the CenterName? When printing in the logs, I think it is more human readable to read the CenterName than an ID referring to a site.
@nicolasbrossard thoughts?
Also, one tiny code change suggested below to make my brain happy ;)
Co-authored-by: Cécile Madjar <cecile.madjar@mcin.ca>
|
@cmadjar good point I'll fetch the name instead I did it in another class for the Alias, remember? the place that triggered your OCD :p |
|
@ridz1208 sounds good. Let me know when implemented and I will test it on my MCIN sandbox. In the meantime, I will update my MCIN sandbox so it is ready for testing. Will update to a commit before the CandID refactoring, I am pretty sure that schema change is going to break a lot of things on the MRI pipeline 😨 |
| @@ -605,7 +606,7 @@ =head2 Methods | |||
| ################################################################ | |||
| # make final logfile name without overwriting phantom logs ##### | |||
| ################################################################ | |||
| my $final_logfile = $center_name; | |||
| my $final_logfile = $mri_alias; | |||
There was a problem hiding this comment.
@nicolasbrossard existential question: why is there MRI alias in the log filename?
|
This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 20 days. |
Someone please check I didnt break everything
please review
getCenterNameFromCenterIDI don't speak this language