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

Update flusurv schema and docs with new age, sex, and race groups #1287

Merged

Conversation

nmdefries
Copy link
Contributor

@nmdefries nmdefries commented Sep 15, 2023

Summary:

Update flusurv table schema to support #1278. Also make sure rate_age_5, 6, and 7 are returned.

Necessary for #1247

Prerequisites:

  • Unless it is a documentation hotfix it should be merged against the dev branch
  • Branch is up-to-date with the branch to be merged with, i.e. dev
  • Build is successful
  • Code is cleaned up and formatted

@nmdefries
Copy link
Contributor Author

nmdefries commented Sep 15, 2023

@korlaxxalrok Tentatively tagging you for review. If you're not familiar with these files, let me know and I'll ask around for an alternative! :)

src/ddl/fluview.sql Outdated Show resolved Hide resolved
@nmdefries nmdefries removed the request for review from korlaxxalrok September 18, 2023 14:41
Copy link
Collaborator

@melange396 melange396 left a comment

Choose a reason for hiding this comment

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

looks good, just a few optional things about repeated information across comments in various places

src/ddl/fluview.sql Show resolved Hide resolved
src/acquisition/flusurv/flusurv_update.py Outdated Show resolved Hide resolved
src/acquisition/flusurv/flusurv_update.py Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Sep 27, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Collaborator

@melange396 melange396 left a comment

Choose a reason for hiding this comment

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

looks good!

@melange396
Copy link
Collaborator

This got lost somehow... Im going to merge it now; I just need to remember to run the migration before release deployment!

@melange396 melange396 merged commit 9de7d64 into ndefries/flusurv-new-endpoint May 8, 2024
6 checks passed
@melange396 melange396 deleted the ndefries/flusurv-new-columns branch May 8, 2024 00:15
@melange396
Copy link
Collaborator

aw crap, i wasnt paying attention and i thought this was going to merge into the dev branch... im going to try to revert it and revisit it again when i have a chance to review #1278

@melange396
Copy link
Collaborator

ugh, i made a mess. this PR's status can't be changed from "Merged". I will leave a note on #1278 that it needs to re-merge this (which is now probably best done by reverting #1426)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants