[PATCH] D142726: [Workflow] Add Release Repo sync script

[Konrad Wilhelm Kleine via Phabricator via llvm-commits]

kwk added a comment.

@thieta I hope my comments make some sense.



================
Comment at: llvm/utils/git/sync-release-repo.sh:5
+# GitHub actions.
+# The script should be executed from a repo cloned from release-prs.
+
----------------
Does it make sense to check for this origin?


================
Comment at: llvm/utils/git/sync-release-repo.sh:16
+
+git config remote.upstream.url >&- || git remote add upstream $REPO
+
----------------
If you default to some "random" UUID you don't have to worry about a remote already exsting.

Oh, and since we're at it. Could you explain a bit what origin is for you and what upstream?


================
Comment at: llvm/utils/git/sync-release-repo.sh:41-42
+if ! git diff-index --quiet upstream/$BRANCH; then
+  echo "Changes in upstream - pushing to origin"
+  git push upstream sync-repos:$BRANCH
+fi
----------------
The text says "pushing to origin" but you're pushing to "upstream" is that on purpose? Sorry it is late here and I might not follow everything.


================
Comment at: llvm/utils/git/sync-release-repo.sh:45-52
+# Done - let's clean up
+git switch $CURRENT_BRANCH
+
+if git diff-index --quiet HEAD; then
+  git reset --hard @{u}
+fi
+
----------------
I'm not sure this should be done. I mean afterall this script is called from a github action and why do you care how the state of that repo looks like? All cleanup steps can be removed IMHO


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142726