-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
[skip vbump] upversion to v0.6.0 #248
base: main
Are you sure you want to change the base?
Conversation
Unit Tests Summary 1 files 24 suites 6s ⏱️ Results for commit 55f2651. |
Unit Tests Summary 1 files 24 suites 8s ⏱️ Results for commit f4c273f. ♻️ This comment has been updated with latest results. |
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.
Locally it ran without any problem or NOTE. I mostly checked the vignettes.
- The image on the "Getting started" vignette is cut, and the table columns are hard to read:
It is an svg file, so it can be edited to fix this.
- I also found a couple of screenshots that do not match current output. I think we are adding shinylive instead of screenshots of vignettes whenever possible. Not sure if we want to do that before release too.
Comparing v0.6.0 vs 0.5.0, I see some other changes that might be worth mentioning on NEWS:
- Some log functions went from trace to debug, while some where from log_warn to warning (
resolve.delayed_value_choices()
). - Some data files were removed and it reuses data from other packages (teal.data)
Signed-off-by: Dony Unardi <donyunardi@gmail.com>
I believe you're referring to the "Combining Data Extract with Data Merge" vignette, not "Getting Started" vignette: I don't see what you're seeing: I did shrink my browser width and it was still responsive nicely.
The output changes because we replaced the example data.
Yes, let's do it! I'll take care of this.
Thanks, I'll add it to NEWS. |
Yes, I confused the vignettes name and I was referring to the "Combining Data Extract with Data Merge". |
Addressing this comment: #248 (review) I'm creating a separate PR to merge the vignettes into the main branch first so they can be built before moving forward with the release. ### Summary * Update all vignettes to include shinylive * Update NEWS * Add roxy.shinylive to Suggest For this comment: > while some where from log_warn to warning (resolve.delayed_value_choices()). I decided not to add to NEWS because the affected function is internal.
Signed-off-by: Dony Unardi <donyunardi@gmail.com>
Fixes #234