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.

Sign up for GitHub

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

Jump to bottom

Fix for # 100 - Remove model library and access #130

Merged
merged 9 commits into from
Oct 27, 2023
Merged

Fix for # 100 - Remove model library and access #130

merged 9 commits into from
Oct 27, 2023

Conversation

bahadzie
Copy link
Member

@bahadzie bahadzie commented Oct 9, 2023

No description provided.

@pratikunterwegs
Copy link
Collaborator

pratikunterwegs commented Oct 10, 2023 •
edited
Loading

Thanks @bahadzie, I'll take a look today. Could you close and re-open the PR to trigger the required workflows, and maybe also fix the merge conflict?

@pratikunterwegs
Copy link
Collaborator

Hi @bahadzie, thanks for the PR - could you rebase it on main and resolve the merge conflict?

pratikunterwegs
pratikunterwegs requested changes Oct 25, 2023
View reviewed changes
Copy link
Collaborator

@pratikunterwegs pratikunterwegs left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR @bahadzie - I only have a few comments which I've left on the files. Just FYI I've rebased this on main, and another rebase would probably be necessary soon as I'm merging #128.

For future reference @amanda-minter had asked about where users might be able to find information on the models, including the compartment names, so I do think the model library is something we should reconsider in future; perhaps once there are more models.

.Rbuildignore Outdated Show resolved Hide resolved
.gitignore Show resolved Hide resolved
R/check_args_default.R Outdated Show resolved Hide resolved
R/check_args_ebola.R Outdated Show resolved Hide resolved
tests/testthat/test-ebola_model.R Outdated Show resolved Hide resolved
tests/testthat/test-epidemic_default.R Outdated Show resolved Hide resolved
@pratikunterwegs
Copy link
Collaborator

Just a note on the last commit 9e55bee - we try to rebase on main so as not to have these update commits. Once checks pass, I'll squash-merge this PR rather than rebasing; more on Epiverse merge strategy here in the Blueprints.

pratikunterwegs
pratikunterwegs approved these changes Oct 27, 2023
View reviewed changes
Copy link
Collaborator

@pratikunterwegs pratikunterwegs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing @bahadzie, looks good to me and squash-merging this now.

@pratikunterwegs pratikunterwegs merged commit 7161788 into main Oct 27, 2023
11 checks passed
@pratikunterwegs pratikunterwegs deleted the bahadzie/issue100 branch October 27, 2023 10:19
@pratikunterwegs pratikunterwegs mentioned this pull request Nov 6, 2023
Closed
@pratikunterwegs pratikunterwegs mentioned this pull request Nov 21, 2023
Closed
@amanda-minter amanda-minter mentioned this pull request Nov 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pratikunterwegs pratikunterwegs pratikunterwegs approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bahadzie @pratikunterwegs