Skip to content

ARROW-7801: [Developer] Add issue_comment workflow to fix lint/style/codegen#6932

Closed
nealrichardson wants to merge 10 commits into
apache:masterfrom
nealrichardson:autotune2
Closed

ARROW-7801: [Developer] Add issue_comment workflow to fix lint/style/codegen#6932
nealrichardson wants to merge 10 commits into
apache:masterfrom
nealrichardson:autotune2

Conversation

@nealrichardson

@nealrichardson nealrichardson commented Apr 14, 2020

Copy link
Copy Markdown
Member

#6411 tried to do this work automatically, but some were uncomfortable with that; this version instead is triggered by PR comment.

Currently implemented:

To trigger the build, add a comment with the message "autotune" (seemed appropriate since this is using computers to make sure our code stays in harmony). By default it will check for which files changed and only run the relevant tasks, but if you comment "autotune everything" it will run everything.

Since issue_comment workflows are only run off the master branch, I tested this by merging these commits to master on my fork and triggering it on a PR there. See e.g. nealrichardson#2 (comment)

@github-actions

Copy link
Copy Markdown

@wesm

wesm commented Apr 15, 2020

Copy link
Copy Markdown
Member

Having this as a developer opt-in sounds good to me.

@bkietz bkietz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm looking forward to trying this out!
Looks like CMAKE_FORMAT is disabled, though

Comment on lines +86 to +87
if: false
# if: env.CMAKE_FORMAT == 'true' || endsWith(github.event.comment.body, 'everything')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Debug code?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Per the description:

@bkietz bkietz Apr 16, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oops, sorry I missed that. Could you add a follow up JIRA and a matching todo comment?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🦒 done in 5bfce2d

git config user.name "$(git log -1 --pretty=format:%an)"
git config user.email "$(git log -1 --pretty=format:%ae)"
git commit -a -m 'Autoformat/render all the things [automated commit]' || echo "No changes to commit"
- uses: r-lib/actions/pr-push@master

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's no action from the base library that does this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not afaik, not yet.

@fsaintjacques

Copy link
Copy Markdown
Contributor

autotune, are you ready?

@fsaintjacques

Copy link
Copy Markdown
Contributor

@nealrichardson shouldn't my previous comment trigger something?

@nealrichardson

Copy link
Copy Markdown
Member Author

@fsaintjacques no, it won't, not until this is merged. The PR description says:

Since issue_comment workflows are only run off the master branch, I tested this by merging these commits to master on my fork and triggering it on a PR there. See e.g. nealrichardson#2 (comment)

@nealrichardson nealrichardson changed the title ARROW-7801: [R][CI] Add lint and doc GitHub Action workflows ARROW-7801: [Developer] Add issue_comment workflow to fix lint/style/codegen Apr 16, 2020
@nealrichardson nealrichardson deleted the autotune2 branch April 16, 2020 23:19
kszucs pushed a commit that referenced this pull request Apr 20, 2020
…codegen

#6411 tried to do this work automatically, but some were uncomfortable with that; this version instead is triggered by PR comment.

Currently implemented:

* R documentation (`roxygen2`, updating `r/man/*.Rd` based on docstrings in `r/R/*.R`)
* clang-format on changes to `cpp/src` or `r/src`
* cmake_format is currently disabled because I didn't feel like fighting further with python versions and environments; in https://github.com/nealrichardson/arrow/runs/590355718?check_suite_focus=true#step:5:34 you can see that it pip installs cmake_format but then in the next line can't find cmake_format 🤷

To trigger the build, add a comment with the message "autotune" (seemed appropriate since this is using computers to make sure our code stays in harmony). By default it will check for which files changed and only run the relevant tasks, but if you comment "autotune everything" it will run everything.

Since `issue_comment` workflows are only run off the master branch, I tested this by merging these commits to master on my fork and triggering it on a PR there. See e.g. nealrichardson#2 (comment)

Closes #6932 from nealrichardson/autotune2

Authored-by: Neal Richardson <neal.p.richardson@gmail.com>
Signed-off-by: Neal Richardson <neal.p.richardson@gmail.com>
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