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

Enhancements to README.Rmd #80

Closed
wants to merge 1 commit into from
Closed

Enhancements to README.Rmd #80

wants to merge 1 commit into from

Conversation

bahadzie
Copy link
Member

@bahadzie bahadzie commented Sep 1, 2023

  • Additional Config variables added to DESCRIPTION
  • render_readme.yml workflow reads additional config
  • README.Rmd updated with additional variables

- Additional Config variables added to DESCRIPTION
- render_readme.yml workflow reads additional config
- README.Rmd updated with additional variables
@bahadzie bahadzie enabled auto-merge (rebase) September 1, 2023 20:01
@Bisaloo Bisaloo disabled auto-merge September 4, 2023 07:38
@Bisaloo
Copy link
Member

Bisaloo commented Sep 4, 2023

Thanks for the PR!

I like the idea of automating more the README content but I'm not sure this PR is going in the right direction:

  • While it is allowed to use Config/ in DESCRIPTION for custom config params, I think we should still try to not go too much beyond standards (whether official of de facto community standards) and create a bunch of new fields just for Epiverse.
  • There is value to put it in DESCRIPTION rather than README.Rmd only in the case these fields are re-used in more than one file, which would not be the case here.
  • Since these fields are not standard, I'm afraid it would create confusion for developers about where to look when they want to update, e.g., their package lifecycle in the README. There is a difficult balance to maintain between automation and not having too many ad-hoc processes. I feel we are already generally tilting a bit too much on the side of automation, at the expense of following usual processes and I believe this PR is definitely pushing the cursor too far.

One alternative to what you're proposing would be to:

  • Have a separate config file but it seems a bit overkill and cumbersome
  • Specify these values in README.Rmd, either as rmarkdown parameters, or as variables at the start of the document.

What do you think?

On a tangential note, I would be in favour of removing the "developed at ..." paragraph, which doesn't seem very useful since we already have standard practices and quality control throughout the project. But I can open a separate issue for this.

@Bisaloo
Copy link
Member

Bisaloo commented Oct 9, 2023

Closing this PR as it's going stale. Feel free to follow up in a new issue.

@Bisaloo Bisaloo closed this Oct 9, 2023
@Bisaloo Bisaloo deleted the README_Rmd_variables branch November 30, 2023 12:24
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.

2 participants