Is the double echo unnecessary?

export CHANNEL=$(${{ github.event.label.name }} | cut -d ':' -f 2)

Tested and my understanding is that both the inner and outer echos are needed.

My understanding is that the inner echo is needed to pipe it to the next expression.
without the inner echo, $word | cut -d ':' -f 2 would throw command not found error on $word
with the echo, echo $word | cut -d ':' -f 2 would pipe the output of echo $word into the next expression

The outer echo is needed due to the unique way of storing environment variables in github actions.
without the outer echo and using export $var, the variable is not stored in github actions. failed workflow
with the outer echo, Echo it into GITHUB_ENV would prevserve it. (https://stackoverflow.com/questions/57968497/how-do-i-set-an-env-var-with-a-bash-expression-in-github-actions)

To help with future reviews, do you have a recommended resource on the shell syntax GitHub is using in these run fields?

Umm yeah I haven't found a source that includes all the supported syntax. Currently my learnings come from failed workflows and stackoverflow.

I found https://docs.github.com/en/actions/learn-github-actions/essential-features-of-github-actions which covers the basics

Should this be implemented now?

Sure, created #139590.

The template has to be in markdown format cause the original web form issue template (https://github.com/flutter/flutter/blob/master/.github/ISSUE_TEMPLATE/7_cherry_pick.yml) isn't supported in PR description.

For future reference, I think we could've implemented that PR in this one to keep all the context together.

yeah I implemented separately cause I thought if the template gets merged into flutter/flutter first, then we can directly use it in github actions. otherwise even if they are in the same PR, the github action still has to download the template from somewhere else because the template doesn't exist on framework yet.

Instead of using a PAT, is the plan to follow https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs to grant write access to this job's GITHUB_TOKEN?

Yeah we have the line permissions: write-all.

Good point! did some tests and I think it works on the same repo where the action is installed. e.g. the github.token of flutter/flutter cannot comment on flutter/engine. but should work on flutter/flutter after we merge this PR.

Can we re-use the original commit message?

My understanding is that there isn't an original commit message, because the message associated with a merged commit sha (cp commit sha), is the PR description of the merged PR.

e.g. the merged commit sha is af61c6c of #139348. running the query gh search commits --repo flutter/flutter --hash e5762ba --json commit gives a commit message that is several hundred chars (the pr description)

e.g. in the cherry pick pr #139594. The commit message associated with the cherry picked commit is the original PR title.

That SGTM as that's expected. However, is commit message needed here? It seems to be creating an empty commit.

Sure, removed the commit-message and committer fields.

I think by default their values are set by the market place action to be [create-pull-request] automated change and GitHub <[email protected]> if we omit these fields

Do we need the create pull request action? Can we use the gh cli directly?

Yeah we can definitely use gh cli directly.

My first few drafts are all implemented with pure gh/git commands, but I think in previous design/code reviews it was pointed out that I should use market place actions whenever possible. I am good with reimplementing with gh and git commands again.

However, if the concern here is that an unnecessary commit message will be generated, then I can assure that no duplicate/extra commit message will be produced. See #139843 as an example where I cp-ed a chillers pr using market place action.

Is this needed if below we use "body-path" to use the template?

Yeah i added a TODO here but we can only remove this part after #139590 lands (or when Kevin is back).

Without an existing template in flutter/flutter, we will have to download the template from somewhere and include it in the opened pull request.

With #139590 ready, can we remove the TODO?

Tested around and realized I cannot remove this step because the template isn't on release branches yet.

Currently the template is only on master head and not on release branches. When we are creating the pull request from the release branch, this file path becomes unavailable to us. For now, I still have to download it from the flutter/flutter repo.

To help with future reviews, do you have a recommended resource on the shell syntax GitHub is using in these run fields?

It seems counter-intuitive to do a depth 0 to ensure we have the CP object. Is depth needed in this case? I thought we were doing it for space savings on the runner.

Yeah so fetch-depth means the number of commits to fetch, and fetch-depth: x means fetching the last x commits on branch ref.

For the cherry pick commit, its unclear where the commit is on the master branch. e.g. the cp commit could be located at 201 commits before HEAD, and therefore fetch-depth: 200 would not retrieve the cp commit.

I had to do a fetch-depth:0 here (0 fetches all commits on ref), so that no matter where the cp commit is in the commit history, it is always retrieved and a known object. Even if it is the first commit of the repository.

By default, fetch-depth is set to 1 which only retrieves the tip commit on branch ref.

In that case, updating the docs to say "to checkout the history of master branch" would make this clear. In it's current state, it's still implied it's getting 0 history.

Updated the comment to Checkout all history commits on master branch, so that the cp commit is a known object

I might have misunderstood and I am good with whichever comment that would make this part clearer. If Casey proposes a change here I d be happy to accept it.

That SGTM as that's expected. However, is commit message needed here? It seems to be creating an empty commit.

For future reference, I think we could've implemented that PR in this one to keep all the context together.