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

Rk 621 comm metric fix #622

Merged
merged 12 commits into from
Aug 17, 2023
Merged

Rk 621 comm metric fix #622

merged 12 commits into from
Aug 17, 2023

Conversation

Robert-Krajcik
Copy link
Contributor

@Robert-Krajcik Robert-Krajcik commented Aug 15, 2023

Addresses issue #621

Looks like the initialize_metric_table.sql had already been updated.
The "Package Downloads" card is no longer flagged as non-metric

But the metric table in upload_format.database needs updating.

image

@Robert-Krajcik Robert-Krajcik self-assigned this Aug 15, 2023
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #622 (229cb78) into dev (60f0c0c) will decrease coverage by 0.11%.
Report is 1 commits behind head on dev.
The diff coverage is n/a.

❗ Current head 229cb78 differs from pull request most recent head f736cf9. Consider uploading reports for the commit f736cf9 to get more accurate results

@@            Coverage Diff             @@
##              dev     #622      +/-   ##
==========================================
- Coverage   76.44%   76.34%   -0.11%     
==========================================
  Files          29       29              
  Lines        3919     3919              
==========================================
- Hits         2996     2992       -4     
- Misses        923      927       +4     

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Robert-Krajcik
Copy link
Contributor Author

code used to update (and verify) the metric table

test_db_loc <- system.file("testdata", "upload_format.database", package = "riskassessment")
dbSelect("select * from metric ", test_db_loc)

dbUpdate("drop table metric", test_db_loc)
src_code <- readLines(system.file("sql_queries", "create_metric_table.sql", package = "riskassessment"), warn = FALSE)
sql_code <- paste(src_code, collapse = " ")
dbUpdate(sql_code, test_db_loc)
src_code <- readLines(system.file("sql_queries", "initialize_metric_table.sql", package = "riskassessment"), warn = FALSE)
sql_code <- paste(src_code, collapse = " ")
dbUpdate(sql_code, test_db_loc)

dbSelect("select * from metric ", test_db_loc)

@Robert-Krajcik Robert-Krajcik marked this pull request as ready for review August 17, 2023 12:59
Copy link
Collaborator

@AARON-CLARK AARON-CLARK 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 Robert. I just made a few comments below. Could you also increment the dev version? Thanks!

@Robert-Krajcik
Copy link
Contributor Author

I had noticed the weight for covr_coverage was = 1 in the upload_format_database metric table
code used to correct it:

test_db_loc <- system.file("testdata", "upload_format.database", package = "riskassessment")
dbUpdate("update metric set weight = 0 where name = 'covr_coverage'", test_db_loc)
target <- dbSelect("select * from metric", test_db_loc)
compar <- dbSelect("select * from metric", "database.sqlite")
all.equal(target, compar)

Copy link
Collaborator

@AARON-CLARK AARON-CLARK left a comment

Choose a reason for hiding this comment

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

👍🏼 Looks great!

@AARON-CLARK AARON-CLARK merged commit f15691b into dev Aug 17, 2023
@AARON-CLARK AARON-CLARK deleted the rk-621-comm_metric_fix branch August 29, 2023 14:31
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.

2 participants