-
Notifications
You must be signed in to change notification settings - Fork 2
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
(2022 - 2025) Remove Index.tsx from Measures #2539
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i got to look at the script and everything
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the comment about the test. CPA-AD
@@ -146,13 +146,13 @@ describe("test measure floating bar menu", () => { | |||
<RouterWrappedComp> | |||
<MeasureWrapper | |||
measure={ | |||
<AABAD | |||
<CPUAD | |||
name={ | |||
"Avoidance of Antibiotic Treatment for Acute Bronchitis/Bronchiolitis: Age 18 And Older" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to match the name and measureId for CPUAD here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah we probably should for consistency sake. i'll make the changes in a less chonky PR. thanks!
@@ -146,13 +146,13 @@ describe("test measure floating bar menu", () => { | |||
<RouterWrappedComp> | |||
<MeasureWrapper | |||
measure={ | |||
<AABAD | |||
<CPUAD | |||
name={ | |||
"Avoidance of Antibiotic Treatment for Acute Bronchitis/Bronchiolitis: Age 18 And Older" | |||
} | |||
year={"2024"} | |||
measureId={"AAB-AD"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be measureId={"CPU-AD"}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do the same as above for this one as well
302f518
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kudos Ben for catching those changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is outstanding work, and I approve.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve - again 😆
Description
I know this PR is ridiculous and I'm really testing the limits of everyone's faith in me but I can promise that all the measures at least renders.
This is probably the last time I'll have a PR with 1000+ file changes.
Refer to previous pr for the same information as this PR.
Related ticket(s)
CMDCT-4242
How to test
Realistically there is no way to test all this in a reasonable amount of time, but we can lure ourselves into a false sense of assurance by clicking around and making sure the measures load and save.
Deploy Link: https://dcjatu2g4y7s8.cloudfront.net/
Notes
Pre-review checklist
Pre-merge checklist
Review
Security
If either of the following are true, notify the team's ISSO (Information System Security Officer).
convert to a different template: test → val | val → prod