Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Feature]: consider having doc-and-style use "commit-directly: true" by default #136

Closed
iantaylor-NOAA opened this issue Aug 2, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@iantaylor-NOAA
Copy link
Collaborator

Is your feature request related to a problem? Please describe.

@e-perl-NOAA added commit-directly: true to the r4ss workflow for doc-and-style following a suggestion from @k-doering-NOAA and I love it: https://github.com/r4ss/r4ss/blob/spr_split_tables/.github/workflows/call-doc-and-style-r.yml#L21.
The changes was made after I messed up the workflow by naively having it apply to pull requests (which would have required an extra pull-request to the pull request). See also #135 and discussion here: NOAA-FIMS/FIMS#660.

Since 2021 I've now merged exactly 100 pull requests created by the doc-and-style workflow in r4ss, and never NOT been happy with the changes. Using commit-directly: true avoids all the extra work and notifications associated with merging a pull request while still providing plenty of opportunity to review the changes by looking at the commit that the GHA adds.

Describe the solution you would like.

Either add commit-directly: true to the workflow or give the user an argument to choose to add it. If there are branch protections, then committing directly to main would be a problem, but in that case everything will come through a pull request and the changes could be committed directly to the branch in the PR.

Describe alternatives you have considered

Keep the status-quo workflows and add a note in the documentation that the user could easily make the change manually.

Additional context

No response

@iantaylor-NOAA iantaylor-NOAA added enhancement New feature or request triage_needed This issue needs review labels Aug 2, 2024
@k-doering-NOAA
Copy link
Collaborator

Thanks for the feedback, @iantaylor-NOAA !

I think this option is included in the use_doc_and_style_r() function, but it is not well advertised. Here's some thoughts on how to make it more well known:

  1. Specifically list the commit-directly option on the readme.
  2. Add examples to the documentation showing how to set up the commit-directly option
  3. We could change the defaults if others agree. I personally like the ability to look over a PR and it is a more conservative option to start out with - but perhaps others would agree this would be better!

@k-doering-NOAA k-doering-NOAA removed the triage_needed This issue needs review label Aug 2, 2024
@k-doering-NOAA
Copy link
Collaborator

Also, I opened #137 for the pull request problem you experienced, thank you for reporting!

@iantaylor-NOAA
Copy link
Collaborator Author

The how_to_commit argument is totally fine. I'm sorry I didn't take the time to look at the existing options.
Based on the dependency graph, I think we could just use word of mouth to spread the idea of using that switch, so feel free to close this issue.

k-doering-NOAA added a commit that referenced this issue Aug 2, 2024
k-doering-NOAA added a commit that referenced this issue Aug 2, 2024
* add error msg for creating prs on prs in doc and style
Addresses #137

* add doc and style examples
addresses #136

* fix: broken code

* fix: doc mistake
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants