-
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
Add round_id expected pattern match check #88
Conversation
@zkamvar , I'm getting some weird test failures surfacing from what I think is strange behaviour of Note that I don't get these failures locally |
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 had a couple of very minor comments.
Thanks for the review @elray1 ! I responded to your questions, added some comments and additional info and also asked for some help on thinking through something. |
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.
one minor indentation fix required to satisfy the linter
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.
approved; tests failing for unrelated reasons
A weird bug just popped up and I don't know where it's coming from, but effectively, when a value with a class of "error" is passed to `encodeString()`, it fails with: Error in UseMethod("conditionMessage") : no applicable method for 'conditionMessage' applied to an object of class "error" This never used to happen, so I'm fixing it here.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #88 +/- ##
==========================================
+ Coverage 89.24% 89.25% +0.01%
==========================================
Files 29 30 +1
Lines 2185 2420 +235
==========================================
+ Hits 1950 2160 +210
- Misses 235 260 +25 ☔ View full report in Codecov by Sentry. |
In schema v4.0.1 we are formalising the requirement of round_id values to match the pattern of either being an ISO formatted date (
YYYY-MM-DD
) or alphanumerics separated by underscores (_
).This PR resolves #68 by:
round_id
variable match the expected pattern match check when validating config.round_id
argument value whenround_id_from_variable = FALSE
or the values in theround_id
variableround_id_from_variable = TRUE
when creatinground
objects programmatically.Note that a large portion of the PR is new example config files.
Also note that I know there are some unrelated failing tests that I'm hoping to get some help with but would be great to get some feedback on the contents of the PR to keep things moving.
Note: unrelated test failures still exist and will need resolving before mergingFound solution @zkamvar had applied in #86 and cherry picked into here!