-
Notifications
You must be signed in to change notification settings - Fork 7
Prepare for v3 #154
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
Prepare for v3 #154
Conversation
* The new columns are used to provide versioning information for each variable. * The version column is set to 2.2.0 which is the current version of the package * The lastUpdated column is set to the date in the v3.0.0 branch. The date does not really matter for this commit. * The status column is set to active.
The default value was bought over from the v3.0.0 branch
The columns now match up with what's in the v3.0.0 branch which should improve git diffs and make it easier to review changes
The column order now matches up with what's in the v3.0.0 branch which should improve diffs and make it easier to review changes.
9a82075 to
8e43bb5
Compare
This CEP proposes a standardization tool for variables.csv and variable_details.csv to ensure consistent formatting across different editors and operating systems, enabling clean semantic diffs in version control.
9792e8a to
642fc30
Compare
DougManuel
left a comment
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.
A few comments:
- Potentially missing columns - The YAML schemas in inst/metadata/schemas/core/variables.yaml include
notesin the expected order, and variable_details.yaml includestemplateVariableas an optional field.
Should these be added?
- If you remember, I drafted a similar validation for my work-in-progress. That is in feature/csv-standardisation-updates. Here is a comparison of features, in case you wanted to look that the validations that I included. The pattern, enum and cross-fields were errors that I was dealing with when reviewing and creating code.
| Feature | csv-standardisation-updates |
This PR |
|---|---|---|
| Column order validation | ✅ | ✅ |
| Row sorting validation | ❌ | ✅ |
| Line ending validation (LF/CRLF) | ✅ | ✅ |
| Excessive quoting detection | ❌ | ✅ |
| Trailing empty columns | ❌ | ✅ |
| Pattern validation (regex) | ✅ | ❌ |
| Enum validation | ✅ | ❌ |
| Cross-field validation | ✅ | ❌ |
| Auto-fix capability | ✅ | ✅ |
| GitHub Action | ❌ | ✅ |
- I validated using the rules in The YAML schemas in inst/metadata/schemas/core/variables.yaml for column names, ordering, etc. dynamically, rather than hardcoding. The "source of truth" is an interesting discussion.
I also drafted a more extensive report inspired by pkgdown and devtools. I like your error reporting very much, but take a look that that branch if you are interested.
Minor Items
- There's a capture.output(print(row_being_checked), file = "log.txt", append = TRUE) in recode-with-table.R:932 that looks like debug code - should that be removed?
- readr is used in fix_worksheet() but isn't in DESCRIPTION. I think the validation should be a pernament feature of cchsflow, so adding that to description is a consideration.
|
@DougManuel Thanks for the review! Couple replies below,
|
1c98e77 to
a666706
Compare
…for formatting issues Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tests into its own file
For sure, keep this PR focused. I've actually been creating more checks as I encounter issues, but that is for the future. |
Agree. Feel free to close the PR. |
…variable details sheet Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…details sheets for formatting issues
When recoding derived variables the function was was using the wrong row to extract the name of the custom function name
…ions, `check_worksheet` and `fix_worksheet`
The purpose of this PR is to prepare the dev branch for merging the changes in from the v3.0.0 branch. To that end it does the following: