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

Rename Quality control and data team helper to imaging/behavioural QC and reorganise menu #5620

Merged
merged 5 commits into from
Nov 13, 2019
Merged

Rename Quality control and data team helper to imaging/behavioural QC and reorganise menu #5620

merged 5 commits into from
Nov 13, 2019

Conversation

ridz1208
Copy link
Collaborator

@ridz1208 ridz1208 commented Nov 13, 2019 •
edited
Loading

Brief summary of changes

  • removed (hopefully) all reference to "data team helper" and replaced with Behavioural Quality Control
  • removed bahavioural tab from QC module and renamed the module to Imaging Quality Control
  • reoganised menu
    • Imaging QC -> imaging tab
    • Behavioural QC -> clinical tab
    • genomic browser -> (new) genomics tab

Full module tests should be run and code grepped to ensure nothing is left behind

@maltheism
Copy link
Member

I'm referencing #5330 and to help me refactor more easily.

@ridz1208 ridz1208 changed the title 2019 11 12 module rename Rename Quality control and data team helper to imaging/behavioural QC and reorganise menu Nov 13, 2019
@ridz1208
Copy link
Collaborator Author

@maltheism sorry about this... orders from above

@ridz1208 ridz1208 closed this Nov 13, 2019
@ridz1208 ridz1208 reopened this Nov 13, 2019
@ridz1208 ridz1208 closed this Nov 13, 2019
@ridz1208 ridz1208 reopened this Nov 13, 2019
@ridz1208 ridz1208 added Cleanup PR or issue introducing/requiring at least one clean-up operation Critical to release PR or issue is key for the release to which it has been assigned 22.0.0 TESTING labels Nov 13, 2019
driusan
driusan approved these changes Nov 13, 2019
View reviewed changes
Copy link
Collaborator

@driusan driusan left a comment

Choose a reason for hiding this comment

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

It looks fine, I'm good with merging this and letting the testing round shake out anything that was missed.

@driusan driusan merged commit e71be47 into aces:22.0-release Nov 13, 2019
@christinerogers christinerogers mentioned this pull request Nov 13, 2019
Open
@christinerogers
Copy link
Contributor

I think the top-line menu change should have been issued separately because it's a big change in the cosmetics of the top menu bar.

Tagging needs documentation
There's a Readme coming for this module from @knoll-alex - fyi @LeighMac - see Alex's PR on her fork which will be updated accordingly and rebased

@christinerogers christinerogers added the State: Needs documentation PR that needs a more exhaustive documentation to proceed label Nov 13, 2019
christinerogers
christinerogers reviewed Nov 13, 2019
View reviewed changes
modules/behavioural_qc/test/behavioural_qcTest.php
@@ -62,10 +62,10 @@ function testDataTeamHelperWithoutPermission()
*
* @return void
*/
function testDataTeamHelperPermission()
function testBeahaviouralQCPermission()
Copy link
Contributor

Choose a reason for hiding this comment

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

typo : Beahav

@knoll-alex knoll-alex mentioned this pull request Nov 13, 2019
Merged
@ridz1208 ridz1208 added this to the 22.0.0 milestone Nov 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@christinerogers christinerogers christinerogers left review comments

@driusan driusan driusan approved these changes

Assignees
No one assigned
Labels
Cleanup PR or issue introducing/requiring at least one clean-up operation Critical to release PR or issue is key for the release to which it has been assigned State: Needs documentation PR that needs a more exhaustive documentation to proceed
Projects
None yet
Milestone
22.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@ridz1208 @maltheism @christinerogers @driusan