fix merge upstream workflow - #106
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds error handling to the git push commands in modular_nemesis/tools/merge_upstream.sh to provide a descriptive error message when the push fails due to insufficient token permissions. The reviewer suggested refactoring the conditional block to eliminate code duplication by setting the push options dynamically and executing the checkout and push commands once after the conditional block.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if git ls-remote --exit-code --heads origin "${MERGE_BRANCH}" >/dev/null 2>&1; then | ||
| log "Branch ${MERGE_BRANCH} exists, resetting it to upstream/${UPSTREAM_BRANCH}" | ||
| git checkout -f -B "${MERGE_BRANCH}" "upstream/${UPSTREAM_BRANCH}" | ||
| git push --force origin "${MERGE_BRANCH}" | ||
| git push --force origin "${MERGE_BRANCH}" || die "failed to push ${MERGE_BRANCH}; the token needs 'contents' AND 'workflows' write (upstream changes .github/workflows/* files)" | ||
| else | ||
| log "Creating branch ${MERGE_BRANCH} from upstream/${UPSTREAM_BRANCH}" | ||
| git checkout -f -B "${MERGE_BRANCH}" "upstream/${UPSTREAM_BRANCH}" | ||
| git push -u origin "${MERGE_BRANCH}" | ||
| git push -u origin "${MERGE_BRANCH}" || die "failed to push ${MERGE_BRANCH}; the token needs 'contents' AND 'workflows' write (upstream changes .github/workflows/* files)" | ||
| fi |
There was a problem hiding this comment.
🟡 Рекомендация по улучшению структуры кода
Приветствую! Я проанализировала предложенные Вами изменения.
Проблема:
В текущей реализации наблюдается дублирование кода в блоках if и else:
- Команда
git checkout -f -B "${MERGE_BRANCH}" "upstream/${UPSTREAM_BRANCH}"выполняется в обеих ветках. - Длинное сообщение об ошибке в
die "..."для командыgit pushполностью дублируется.
Это усложняет чтение кода и его дальнейшую поддержку.
Решение:
Рекомендуется вынести общую логику (команды checkout и push) за пределы условного оператора, а в самом условии определять только специфичный флаг для отправки изменений (--force или -u). Это сделает скрипт более лаконичным и избавит от дублирования.
| if git ls-remote --exit-code --heads origin "${MERGE_BRANCH}" >/dev/null 2>&1; then | |
| log "Branch ${MERGE_BRANCH} exists, resetting it to upstream/${UPSTREAM_BRANCH}" | |
| git checkout -f -B "${MERGE_BRANCH}" "upstream/${UPSTREAM_BRANCH}" | |
| git push --force origin "${MERGE_BRANCH}" | |
| git push --force origin "${MERGE_BRANCH}" || die "failed to push ${MERGE_BRANCH}; the token needs 'contents' AND 'workflows' write (upstream changes .github/workflows/* files)" | |
| else | |
| log "Creating branch ${MERGE_BRANCH} from upstream/${UPSTREAM_BRANCH}" | |
| git checkout -f -B "${MERGE_BRANCH}" "upstream/${UPSTREAM_BRANCH}" | |
| git push -u origin "${MERGE_BRANCH}" | |
| git push -u origin "${MERGE_BRANCH}" || die "failed to push ${MERGE_BRANCH}; the token needs 'contents' AND 'workflows' write (upstream changes .github/workflows/* files)" | |
| fi | |
| if git ls-remote --exit-code --heads origin "${MERGE_BRANCH}" >/dev/null 2>&1; then | |
| log "Branch ${MERGE_BRANCH} exists, resetting it to upstream/${UPSTREAM_BRANCH}" | |
| push_opts="--force" | |
| else | |
| log "Creating branch ${MERGE_BRANCH} from upstream/${UPSTREAM_BRANCH}" | |
| push_opts="-u" | |
| fi | |
| git checkout -f -B "${MERGE_BRANCH}" "upstream/${UPSTREAM_BRANCH}" | |
| git push ${push_opts} origin "${MERGE_BRANCH}" || die "failed to push ${MERGE_BRANCH}; the token needs 'contents' AND 'workflows' write (upstream changes .github/workflows/* files)" |
References
- Раздел 4.1 репозиторного стайлгайда рекомендует выявлять дублирование кода и устранять повторяющиеся блоки для улучшения читаемости и поддержки. (link)
Информация о PR
bruh