Skip to content

doocument() backport_linter #694

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

Merged
merged 2 commits into from
Dec 7, 2020

Conversation

AshesITR
Copy link
Collaborator

@AshesITR AshesITR commented Dec 6, 2020

backport_linter was missing documentation.

  • added doc for argument r_version
  • run document()

@MichaelChirico
Copy link
Collaborator

MichaelChirico commented Dec 6, 2020 via email

@AshesITR
Copy link
Collaborator Author

AshesITR commented Dec 6, 2020

You mean a step that would on-push run devtools::document() and commit all changes?

@dragosmg
Copy link
Collaborator

dragosmg commented Dec 6, 2020

@MichaelChirico @AshesITR usethis:use_github_action_pr_commands()? it adds this workflow which runs roxygen2::roxygenise().

@AshesITR
Copy link
Collaborator Author

AshesITR commented Dec 6, 2020

That would require the author to remember to include /document (or /style) in the commit message.
Not sure how much benefit that provides compared to remember running devtools::document() and committing the resulting changes...

NB in the discussion around #584 we explicitly decided against using styler to do a bulk re-styling of the current codebase.

@MichaelChirico
Copy link
Collaborator

That would require the author to remember to include /document (or /style) in the commit message.

What I had in mind is the GHA runner doing it and pushing, no input needed. Probably just need to run this on push to master, otherwise there may be a permissions issue for getting PRs from forks.

@MichaelChirico MichaelChirico merged commit 646313e into master Dec 7, 2020
@MichaelChirico MichaelChirico deleted the fix/doc-backport_linter-r_version branch December 7, 2020 01:04
@dragosmg
Copy link
Collaborator

dragosmg commented Dec 7, 2020

@AshesITR @MichaelChirico I was thinking along the same lines - i.e. not use {styler} and look into configuring the workflow to automatically run (without a key word). I don't mind looking into it if still needed.

@AshesITR
Copy link
Collaborator Author

AshesITR commented Dec 7, 2020

Sounds useful on PRs.
But we should be careful not to break builds accidentally.
document() on master without this pr (especially the @param r_version part) would have caused all builds to fail.

On the other hand, the original PR would have failed if the action was active.
That said, we'll probably need to only be very carefull when rolling this out.

AshesITR added a commit that referenced this pull request Dec 11, 2020
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.

3 participants