[PATCH] D117386: github: Add actions to automate part of the release workflow

[Tom Stellard via Phabricator via llvm-commits]

tstellar marked an inline comment as not done.
tstellar added a comment.

Thanks for the feedback.  I will work on these suggestions.



================
Comment at: .github/workflows/issue-release-workflow.yml:24
+        run: |
+          pip install PyGithub GitPython
+          branch=`./llvm/utils/git/github-automation.py --token ${{ github.token }} release-workflow --issue-number ${{ github.event.issue.number }} print-release-branch`
----------------
kwk wrote:
> I highly suggest to use a `requirements.txt` file instead. I suggest you create a `requirements.txt.in` with this content:
> 
> ```
> # Convert this file into a requirements.txt file by running:
> #
> # pip install pip-tools
> # pip-compile -o requirements.txt  requirements.txt.in
> 
> PyGithub==1.55
> GitPython==3.1.26
> ```
> 
> Then you convert this file locally into a requirements.txt file by running:
> 
> ```
> pip install pip-tools
> pip-compile -o requirements.txt  requirements.txt.in
> ```
> 
> Find out more about requirements.txt here: https://pip.pypa.io/en/stable/reference/pip_install/#requirements-file-format
> 
> One of the added benefits is that you can more easily see when your code is affected by any CVEs and you can maintain direct and indirect dependencies separately. Also local execution of any python code can happen without looking at this nasty github workflow file.
> 
> Of course, version both files, `requirements.txt.in` and `requirements.txt`.
> 
> In this step things would then looks like this:
> 
> ```lang=diff
> - pip install PyGithub GitPython
> + pip install -r requirements.txt
> ```
> 
How important is it to have the version numbers in requirements.txt ?  I'm concerned that the versions available in the github actions runner will change often and that will break this workflow if we have explicit versions.


================
Comment at: .github/workflows/issue-release-workflow.yml:47
+        run:  |
+          curl -O -L https://raw.githubusercontent.com/$GITHUB_REPOSITORY/$GITHUB_SHA/llvm/utils/git/github-automation.py
+          chmod a+x github-automation.py
----------------
kwk wrote:
> Why curl the file instead of cloning like before? If `github-automation.py` happens to need another file from the repo you'd have to remember to patch this place.
I mainly did this because cloning is so slow.  I agree cloning would be better.  I will try to benchmark with a shallow clone to see how long it takes.


================
Comment at: .github/workflows/issue-release-workflow.yml:49
+          chmod a+x github-automation.py
+          pip install PyGithub GitPython
+
----------------
kwk wrote:
> Another benefit of `requirements.txt` is that you have to maintain dependencies in one place only.
That would be nice.  I considered putting this step into a reusable workflow, so it could be more easily shared, but I wasn't sure it was worth it for something so small.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117386/new/

https://reviews.llvm.org/D117386