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

Documentation: Explain subject_keys option in programming strategy #133

Closed
rossfarrugia opened this issue Oct 19, 2022 · 3 comments · Fixed by #152
Closed

Documentation: Explain subject_keys option in programming strategy #133

rossfarrugia opened this issue Oct 19, 2022 · 3 comments · Fixed by #152
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@rossfarrugia
Copy link
Collaborator

Please select a category the issue is focused on?

Developer Guides

Let us know where something needs a refresh or put your idea here!

In admiraldev programming strategy vignette add a sentence to cover the decisions from pharmaverse/admiral#1338 so that anyone creating a new function knows to follow this convention.

@rossfarrugia rossfarrugia added documentation Improvements or additions to documentation release Q4-2022 labels Oct 19, 2022
@bundfussr bundfussr transferred this issue from pharmaverse/admiral Oct 19, 2022
@zdz2101
Copy link
Collaborator

zdz2101 commented Oct 27, 2022

Do you have a recommendation where this snippet should be placed? Perhaps under Input, Output, and Side-effects?

Thinking we might need more than a sentence, maybe something along the lines of this:

Image

@rossfarrugia
Copy link
Collaborator Author

@bundfussr any suggestions?? i'm personally ok with using the Input, Output, and Side-effects section.

i like your suggested wordings and extra detail @zdz2101 and this will act as a good reminder if ever we do need to come back and extend this later.

i'd also suggest we add subject_keys to the table at https://pharmaverse.github.io/admiraldev/devel/articles/programming_strategy.html#list-of-common-parameters reminding developers of this default value here - for me that's the most important reminder as i bet many new functions will get developed and people will forget to use this default (although hopefully if they just copy another function as a starting point they won't anyway).

@bundfussr
Copy link
Collaborator

@zdz2101 , @rossfarrugia , I would

  • add subject_keys to the table as Ross suggested,
  • update the last item in "Input, Output, and Side-effects" as Zelos suggested ("In general, the function must not have any side-effects ..."),
  • move the other two items to a separate top level section ("admiral Options").

zdz2101 pushed a commit that referenced this issue Nov 1, 2022
@zdz2101 zdz2101 linked a pull request Nov 1, 2022 that will close this issue
14 tasks
@bms63 bms63 closed this as completed in dfdc9a4 Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
4 participants