Skip to content

rename foo (#228)#229

Merged
nanxstats merged 2 commits intomainfrom
foo-name
Jun 15, 2023
Merged

rename foo (#228)#229
nanxstats merged 2 commits intomainfrom
foo-name

Conversation

@elong0527
Copy link
Copy Markdown
Collaborator

Address suggestion in #228

Comment thread R/expected_time.R Outdated
# ----------------------------#
# find the difference between `AHR()` and different values of total_duration
foo <- function(x) {
ahr_diff <- function(x) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

shall we call it event_diff? Since basically it calculates the difference between the targeted events and the events at a calendar time...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

sure updated.

Copy link
Copy Markdown
Collaborator

@LittleBeannie LittleBeannie left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Copy Markdown
Contributor

@nanxstats nanxstats left a comment

Choose a reason for hiding this comment

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

I would appreciate it if you could make one small update: moving their function definitions outside of the exported functions.

@fb-elong
Copy link
Copy Markdown

I would appreciate it if you could make one small update: moving their function definitions outside of the exported functions.

Those functions are intentionally defined within a function. Because the only use case is to build a function that fit into root solving functions.

How about we merge this PR and discuss the topic separately in #235?

@nanxstats nanxstats merged commit ac2e384 into main Jun 15, 2023
@nanxstats nanxstats deleted the foo-name branch June 15, 2023 02:31
@nanxstats
Copy link
Copy Markdown
Contributor

Sure! Thanks for the renaming effort and suggestions.

@nanxstats nanxstats mentioned this pull request Jun 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants