You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I discovered an issue with the optimize-bot as seen in #610. The checkout and commit action acts differently when the PR is from a forked repo.
The commit action calls for the checkout action to checkout the github.head_ref when the trigger is pull_request. Unfortunately, this cause the action to checkout the PR branch's name but look for it in the devicons/devicon repo => does not exist. The problem is described here.
Removing the github.head_ref would cause the commit action to fail entirely but everything else works.
I'd like to ask you guys what you think of the following potential solutions:
Changing the commit action into a create-pr action. This is similar to what we already have in the build-fonts action.
The target will be the PR branch:
Pro: Don't need to close the branch and keep everything in the same PR
Con: Might need permission of the owner to merge. If the owner has abandoned the PR, this can be problematic. However, if the owner has abandoned the PR, we'll have to take it over nonetheless and can PR into our own branch.
The target will be develop:
Pro: We can merge it in whenever.
Con: We'll have to close the old PR manually. PR will also be cluttered
Keep the commit action but rather than using files-changed action, manually search for files added based on the PR title.
Pro: Easy to do. peek-bot is using this.
Con: Hacky. Depends on PR title to be correct. Will need a wrapper Python script to run the npm script but that's not too bad.
Run the optimize script another way: on merge, when building, etc...
Pro: Batch things together and save time
Con: need more work.
I'm thinking of doing option 2. We haven't have an issue with peek-bot yet. While it's a bit hacky, it works well and I won't have to do much to add this functionality.
What do you guys think?
The text was updated successfully, but these errors were encountered:
Ok, after doing some work, I realized that option 2 does not work. I misunderstood/did not comprehend the docs for the commit action. Turns out that we will need to make the contributor enables the Action on their forked repo. See this for more details.
I'm thinking that optimizing the SVGs all at once when we do a release (option 3) might be better idea. Either that or I look into the PR action (option 1). I would like to optimize the SVGs right away so people can start using the SVGs in develop ASAP. However, I don't like the cons of option 1 so I'm leaning towards option 3.
Hello guys,
I discovered an issue with the
optimize-bot
as seen in #610. Thecheckout
andcommit
action acts differently when the PR is from a forked repo.The
commit
action calls for thecheckout
action to checkout thegithub.head_ref
when the trigger ispull_request
. Unfortunately, this cause the action to checkout the PR branch's name but look for it in thedevicons/devicon
repo => does not exist. The problem is described here.Removing the
github.head_ref
would cause thecommit
action to fail entirely but everything else works.I'd like to ask you guys what you think of the following potential solutions:
Changing the
commit
action into acreate-pr
action. This is similar to what we already have in thebuild-fonts
action.develop
:Keep the
commit
action but rather than usingfiles-changed
action, manually search for files added based on the PR title.peek-bot
is using this.Run the
optimize
script another way: on merge, when building, etc...I'm thinking of doing option 2. We haven't have an issue with
peek-bot
yet. While it's a bit hacky, it works well and I won't have to do much to add this functionality.What do you guys think?
The text was updated successfully, but these errors were encountered: