Conversation
seabbs
left a comment
There was a problem hiding this comment.
This is looking nice (no comment about jamming so many things in one PR...). I particularly like the updates to the getting started vignette.
Left a few comments with some suggested changes and spotted a couple of minor issues.
|
Give me a ping if need a help closing this out! |
|
Should be ready for another review (modulo some linting errors that I still want to fix later) |
|
Can you ping me when ready for a review please. |
Codecov Report
@@ Coverage Diff @@
## main #299 +/- ##
==========================================
+ Coverage 90.78% 90.87% +0.09%
==========================================
Files 22 22
Lines 1378 1392 +14
==========================================
+ Hits 1251 1265 +14
Misses 127 127
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
ping ping, ready for review. There are some open linting issues which I left open for now as addressing them would likely generate more merge conflicts with #305 |
|
so would #305 then become 1.2.1? Or should this wait for that to be merged? |
|
I would have thought to merge #305 first and then merge this one and make it a release |
seabbs
left a comment
There was a problem hiding this comment.
Looks mostly good to me. I think this needs to wait for #305 to be merged before it is (so we can check linting issues are resolved and because the README will need another refresh when that happens).
Main pain points are confusion about the new is method (is it a method or is it a normal function) and the versioning of changes in this release seem confused.
8d4c33b to
21d92fe
Compare
|
I've rebased on |
|
The release note is very nice, thanks so much! |
This PR
check_forecasts()andscore()pipeable