JuliaDiff / ChainRulesCore.jl

AD-backend agnostic system defining custom forward and reverse mode rules. This is the light weight core to allow you to define rules for your functions in your packages, without depending on any particular AD system.
Other
252 stars 62 forks source link

Formatter Action doesn't post comments when made from a fork #489

Open oxinabox opened 2 years ago

oxinabox commented 2 years ago

When a PR is made from a fork the Formatter failed by doesn't post comments with clickable suggestions: see: https://github.com/JuliaDiff/ChainRulesCore.jl/pull/488/checks?check_run_id=3854350212#step:5:187

reviewdog: This GitHub token doesn't have write permission of Review API 1, so reviewdog will report results via logging command 2 and create annotations similar to github-pr-check reporter as a fallback.

I am not sure if we can raise the permission on the GITHUB_TOKEN on a fork, or if we can issue another token that has that permission?

But checks that fail without being easy to act upon suck. This isn't super hard to do locally, I think it is just

using Pkg: @pkg_str
pkg"activate --temp"
pkg"add JuliaFormatter"
using JuliaFormatter; format("."; verbose=true)

But that is still work.

And especially for PR from newcomers (not @mcabbott :joy: ) who will always be making a PR from a fork, we want things to be smooth.

st-- commented 2 years ago

In case it's not easy to figure out how to get the suggestions on fork PRs: how about providing Makefiles for this - e.g. to allow make format (make is available on any unix-style system) to simplify the process ... would that be something you could see helping new contributors? (if not I'll be forevermore quiet on this topic; I've previously&elsewhere suggested this and it did not seem well-received - "just use VSCode" ...:))

devmotion commented 2 years ago

Didn't want to chime in here initially, but I still think it's more accessible if one provides instructions for how to format the code with Julia (and possibly the most common editor for Julia): not everyone uses a unix system or is familiar with the commandline or make but I think one can assume that every contributor knows how to start Julia (or the editor) and run Julia code.

(BTW I assume you saw that the required changes are still displayed in a somewhat nice way in the "Files" tab, although of course it's still not possible to just accept these changes)

devmotion commented 2 years ago

I wonder if it would be sufficient to use https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows#pull_request_target instead of pull_request or if this would break the reviewdog action. If it doesn't work it would be useful if reviewdog would add support for pull_request_target if possible.