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

898 save app state version 3 #372

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

chlebowa
Copy link
Contributor

@chlebowa chlebowa commented Mar 18, 2024

Companion to insightsengineering/teal#1011
Replaces uses of update*Input with server-side input creation with renderUI.

Copy link
Contributor

github-actions bot commented Mar 18, 2024

badge

Code Coverage Summary

Filename              Stmts    Miss  Cover    Missing
------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------
R/adtteSpec.R           170     123  27.65%   253-401
R/assaySpec.R            54      40  25.93%   104-148
R/barplot.R             188     152  19.15%   39-66, 129-281
R/boxplot.R             190     159  16.32%   40-67, 125-280
R/checkmate.R            18       9  50.00%   110-118
R/experimentSpec.R       90      57  36.67%   97, 215-283
R/forestplot.R          214     186  13.08%   58-92, 152-328
R/geneSpec.R            257     154  40.08%   154-169, 296-482
R/km.R                  208     174  16.35%   61-92, 156-327
R/pca.R                 365     282  22.74%   34-56, 165-476
R/quality.R             317     244  23.03%   18-110, 208-452
R/sampleVarSpec.R       236     103  56.36%   301, 320-328, 334-341, 343, 351-363, 365-366, 368, 371, 379-383, 385-400, 405-429, 432-436, 438, 445-455, 457-458, 466, 511-528
R/scatterplot.R         181     148  18.23%   39-65, 127-274
R/utils.R                16       0  100.00%
R/volcanoplot.R         204     174  14.71%   34-57, 113-294
R/zzz.R                   2       2  0.00%    2-3
TOTAL                  2710    2007  25.94%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  -------
R/assaySpec.R           +1      +1  -0.49%
R/barplot.R             +1      +1  -0.10%
R/boxplot.R             +1      +1  -0.09%
R/forestplot.R          +1      +1  -0.06%
R/geneSpec.R            +1      +1  -0.16%
R/km.R                  +1      +1  -0.08%
R/pca.R                 -1      -1  +0.06%
R/quality.R             -2      -2  +0.14%
R/sampleVarSpec.R        0      -1  +0.42%
R/scatterplot.R         +1      +1  -0.10%
R/volcanoplot.R         +1      +1  -0.07%
TOTAL                   +5      +4  -0.01%

Results for commit: 9be50ff

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Mar 18, 2024

Unit Tests Summary

 1 files  15 suites   8s ⏱️
56 tests 43 ✅ 13 💤 0 ❌
94 runs  81 ✅ 13 💤 0 ❌

Results for commit 9be50ff.

♻️ This comment has been updated with latest results.

@chlebowa
Copy link
Contributor Author

Thank you @danielinteractive.

Copy link
Contributor

github-actions bot commented Mar 20, 2024

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
adtteSpec 💚 $12.01$ $-4.18$ $-444$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
adtteSpec 💚 $8.26$ $-2.20$ h_km_mae_to_adtte_function_works_as_expected_with_a_single_gene

Results for commit 7c7d13e

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented May 7, 2024

CLA Assistant Lite bot:
🎉 Thank you for your contribution! Before this PR can be accepted, we require that you all read and agree to our Contributor License Agreement.
You can digitally sign the CLA by posting a comment on this Pull Request in the format shown below. This agreement will apply to this PR as well as all future contributions on this repository.


I have read the CLA Document and I hereby sign the CLA


0 out of 3 committers have signed the CLA.
@aleksander Chlebowski
@chlebowa
@danielinteractive
Aleksander Chlebowski seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request

@danielinteractive
Copy link
Collaborator

Hi @kartikeyakirar , should this be merged still or not?

@chlebowa
Copy link
Contributor Author

chlebowa commented May 7, 2024

Aleksander Chlebowski seems not to be a GitHub user.

Preposterous.

@donyunardi
Copy link
Contributor

I have to consult with @gogonzo
Looking at the changes, the update seems harmless.
Is it save to merge this?

Note that we did merge similar PR in tmg:

But we didn't do it in tmc:

@chlebowa
Copy link
Contributor Author

chlebowa commented May 8, 2024

tmg was merged because @gogonzo and I went through all modules to check whether they can or can not be bookmarked. Other module packages have not been rigorously tested from that angle. See here.

@donyunardi
Copy link
Contributor

As discussed, we'll convert this draft for now until we're ready to surface the bookmarkable feature.

@donyunardi donyunardi marked this pull request as draft May 9, 2024 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants