Overhaul tooling#63
Merged
openshift-merge-bot[bot] merged 1 commit intoopenshift-eng:mainfrom Apr 23, 2026
Merged
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
added
the
approved
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
RadekManak
force-pushed
the
overhaul-tooling
branch
from
fa1d197 to
fc76737
Compare
damdo
reviewed
Jun 16, 2025
Contributor
damdo
left a comment
damdo
left a comment
There was a problem hiding this comment.
/approve
Thanks @RadekManak that's a nice update!
I'll add my LGTM once CI is green
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: damdo, RadekManak The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
RadekManak
force-pushed
the
overhaul-tooling
branch
2 times, most recently
from
a4a5175 to
6982d0b
Compare
damdo
reviewed
Jun 26, 2025
Comment on lines
+41
to
+44
| echo "=== Debug Information ===" | ||
| echo "Current working directory: $(pwd)" | ||
| echo "Current user: $(whoami)" | ||
| echo "Current user ID: $(id)" |
Contributor
There was a problem hiding this comment.
IIRC you need to use double dollars in Make to escape shell dollars. e.g. $$(pwd)
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
Contributor
Author
|
/retest |
RadekManak
force-pushed
the
overhaul-tooling
branch
from
6982d0b to
bdb5fb8
Compare
Contributor
Author
|
/retest |
RadekManak
force-pushed
the
overhaul-tooling
branch
from
bdb5fb8 to
ef468e1
Compare
Contributor
Author
|
/retest |
RadekManak
force-pushed
the
overhaul-tooling
branch
2 times, most recently
from
1390ce1 to
2be846c
Compare
Contributor
Author
|
/retest |
RadekManak
force-pushed
the
overhaul-tooling
branch
from
2be846c to
56851ca
Compare
Contributor
Author
|
/retest |
openshift-ci
Bot
added
the
needs-rebase
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughMoved packaging to pyproject.toml with pinned deps and console script; removed legacy setup.py/setup.cfg and extra requirements. Replaced pylint/black/mypy with Ruff + uv in Makefile, added CI build-root Dockerfile and Go pin/update script, and modernized type hints and formatting across code and tests. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
RadekManak
force-pushed
the
overhaul-tooling
branch
from
56851ca to
9850020
Compare
openshift-ci
Bot
removed
the
needs-rebase
openshift-ci
Bot
added
the
needs-rebase
RadekManak
force-pushed
the
overhaul-tooling
branch
from
e34236b to
1f8cc21
Compare
openshift-ci
Bot
removed
the
needs-rebase
Contributor
Author
|
/test all |
RadekManak
force-pushed
the
overhaul-tooling
branch
from
1f8cc21 to
766334a
Compare
Contributor
Author
|
/test all |
RadekManak
force-pushed
the
overhaul-tooling
branch
from
766334a to
07ccc94
Compare
Contributor
Author
|
/test all |
RadekManak
force-pushed
the
overhaul-tooling
branch
2 times, most recently
from
9d7d57c to
2ba130c
Compare
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
tests/test_cli.py (1)
51-56: Avoid mutating shared fixture dict in helper.Line 55 updates
valid_args_dictin place. This can make repeated calls within one test stateful and brittle.Suggested fix
def get_valid_cli_args(valid_args_dict: dict) -> Callable[[dict | None], list[str]]: def _valid_cli_args_getter(extra_args: dict | None = None) -> list[str]: extra_args = extra_args or {} - valid_args = valid_args_dict + valid_args = dict(valid_args_dict) valid_args.update(extra_args) return args_dict_to_list(valid_args)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_cli.py` around lines 51 - 56, The helper get_valid_cli_args currently mutates the shared valid_args_dict by calling valid_args.update(extra_args) inside _valid_cli_args_getter; change it to build a new dict instead (e.g., create a shallow copy of valid_args_dict or use dict unpacking like {**valid_args_dict, **extra_args}) so valid_args is a fresh dict per call, then pass that to args_dict_to_list; update references in get_valid_cli_args and _valid_cli_args_getter accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.openshift-ci/Dockerfile.build_root:
- Around line 8-11: The Dockerfile RUN step that downloads and extracts Go using
GO_VERSION must verify the tarball integrity before extraction: modify the RUN
sequence that uses curl ... go${GO_VERSION}.linux-amd64.tar.gz and tar -C
/usr/local -zxvf - to first download the official checksum (or GPG signature)
for go${GO_VERSION}.linux-amd64.tar.gz, validate the downloaded tarball against
that checksum/signature (e.g., sha256sum or gpg verification), and only on
successful verification proceed to extract to /usr/local; update the commands
around the curl/tar invocation to perform the checksum download, verification,
and fail the build if verification fails.
In `@AGENTS.md`:
- Line 1: The document's first-line heading currently links to
"http://AGENTS.md", producing a broken external URL; update the heading in
AGENTS.md (the first line) to a proper local/relative reference or plain title —
for example change "# [AGENTS.md](http://AGENTS.md)" to either "# AGENTS.md" or
"# [AGENTS.md](./AGENTS.md)" so the link is valid within the repo and no longer
points to an invalid external URL.
- Around line 23-45: The module names in AGENTS.md are using malformed markup
like `\`**rebasebot/bot.py`**`; update the four entries referencing
rebasebot/bot.py, rebasebot/cli.py, rebasebot/github.py, and
rebasebot/lifecycle_hooks.py to valid markdown by removing the stray backslashes
and mismatched asterisks and using proper code formatting (e.g.,
`rebasebot/bot.py`) or bold+code if desired (e.g., **`rebasebot/bot.py`**) so
each module name renders correctly.
In `@Makefile`:
- Around line 13-16: The Makefile currently uses .venv paths (PYTHON_BIN,
conditional on IS_CI) but repo guidance requires a venv target that creates env
and uses env/bin/activate; add a new make target named venv that runs python -m
venv env and ensures env/bin/activate is the intended workflow, update the
PYTHON_BIN variable to reference env/bin/python when IS_CI is false (keep
fallback to python for CI), and replace any other references to .venv (e.g.,
targets around lines 52-60) to use env so all targets (installation, lint, test,
etc.) work with make venv followed by source env/bin/activate.
In `@README.md`:
- Around line 183-187: The README uses mismatched bold and code markers for CLI
flags/env vars (e.g., **--pre-rebase-hook**, **--pre-carry-commit-hook**,
**--post-rebase-hook**, **--pre-push-rebase-branch-hook**,
**--pre-create-pr-hook**) causing broken rendering; locate those token
occurrences (also check the nearby blocks mentioned around lines 249-254 and the
malformed token at line 295) and replace the mixed **...**/backtick usage with
consistent inline code formatting using single backticks (e.g.,
`--pre-rebase-hook`), removing stray bold markers so all flags and env vars
render as code spans.
- Around line 49-61: Update the two fenced code blocks that list GitHub
permissions by adding a language identifier (e.g., "text") after the opening
triple backticks so Markdown linters and renderers treat them as plain text;
specifically modify the block containing "- Contents: Read / - Metadata:
Read-only / - Pull requests: Read & Write" and the block containing "- Contents:
Read & Write / - Metadata: Read-only / - Workflows: Read & Write" to start with
```text instead of just ```.
In `@rebasebot/bot.py`:
- Around line 908-910: The log message in the unknown-title-format branch uses
logging.info with the f-string referencing pull_req.title and has a missing
space between sentences ("format.Keeping"); update that logging.info call to
insert a space (e.g., "format. Keeping the current title.") so the message reads
clearly and preserves pull_req.title usage.
In `@rebasebot/lifecycle_hooks.py`:
- Line 112: The hook-fetch exception messages currently use plain strings with
braces and therefore drop dynamic variables; update the two raise ValueError
calls in the lifecycle_hooks hook-fetch logic so they use proper f-strings (or
.format) to interpolate the dynamic context (e.g., include organization, name,
domain/git_url) into the messages — replace constructs like "Failed to add
remote domain/{organization}/{name}" with f"Failed to add remote
domain/{organization}/{name}" (and likewise for the other message around lines
142-145) so the raised ValueError includes the actual runtime values and chain
the original exception with from e as already done.
In `@tests/test_rebases.py`:
- Line 220: Replace the tuple assignment to args.bot_emails with an actual empty
list so the bot-squash branch isn’t enabled incorrectly: change the assignment
of args.bot_emails from ([],) to [] in the failing test (the variable referenced
is args.bot_emails and the condition that checks len(args.bot_emails) controls
the "bot-squash" branch).
---
Nitpick comments:
In `@tests/test_cli.py`:
- Around line 51-56: The helper get_valid_cli_args currently mutates the shared
valid_args_dict by calling valid_args.update(extra_args) inside
_valid_cli_args_getter; change it to build a new dict instead (e.g., create a
shallow copy of valid_args_dict or use dict unpacking like {**valid_args_dict,
**extra_args}) so valid_args is a fresh dict per call, then pass that to
args_dict_to_list; update references in get_valid_cli_args and
_valid_cli_args_getter accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3b72f085-48c8-4e42-a77f-99b07c0c8e71
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
.openshift-ci/Dockerfile.build_root.pylintrcAGENTS.mdMakefileREADME.mdhack/configure-hooks.shhack/linthack/tests.shpyproject.tomlrebasebot/bot.pyrebasebot/cli.pyrebasebot/github.pyrebasebot/lifecycle_hooks.pyrequirements-hacking.txtrequirements.txtsetup.cfgsetup.pytests/conftest.pytests/test_bot.pytests/test_cli.pytests/test_conflict_policy.pytests/test_rebases.py
💤 Files with no reviewable changes (7)
- requirements-hacking.txt
- setup.cfg
- hack/configure-hooks.sh
- hack/lint
- .pylintrc
- setup.py
- requirements.txt
RadekManak
force-pushed
the
overhaul-tooling
branch
from
2ba130c to
b12933b
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.openshift-ci/Dockerfile.build_root (1)
11-18:⚠️ Potential issue | 🟡 MinorClean package/pip caches in the same layer.
This RUN step still leaves DNF and pip caches behind. Add cleanup to reduce image bloat and stale package metadata exposure.
Suggested patch
RUN dnf install -y tar gzip git make && \ ARCH=$(case "${TARGETARCH:-amd64}" in aarch64) echo "arm64" ;; amd64) echo "amd64" ;; *) echo "${TARGETARCH:-amd64}" ;; esac) && \ GO_SHA256=$(case "${ARCH}" in amd64) echo "${GO_SHA256_AMD64}" ;; arm64) echo "${GO_SHA256_ARM64}" ;; *) echo "Unsupported TARGETARCH: ${TARGETARCH:-amd64}" >&2; exit 1 ;; esac) && \ curl -fLsS -o /tmp/go.tar.gz "https://go.dev/dl/go${GO_VERSION}.linux-${ARCH}.tar.gz" && \ echo "${GO_SHA256} /tmp/go.tar.gz" | sha256sum -c - && \ tar -C /usr/local -xzf /tmp/go.tar.gz go/ && \ rm -f /tmp/go.tar.gz && \ - python -m pip install uv + python -m pip install --no-cache-dir uv && \ + dnf clean all && rm -rf /var/cache/dnf /tmp/*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.openshift-ci/Dockerfile.build_root around lines 11 - 18, The RUN layer that installs packages and Go (the block starting with "RUN dnf install -y tar gzip git make && \\" and the "python -m pip install uv" invocation) leaves DNF and pip caches in the image; update that same RUN chain to run "dnf clean all" and remove DNF caches (e.g., rm -rf /var/cache/dnf /var/cache/yum /var/run/dnf*) after installs, and install Python packages with pip using --no-cache-dir (or run "python -m pip cache purge" / rm -rf ~/.cache/pip) so all package manager caches are deleted in the same layer where GO download and tar extraction occur (keep the existing rm -f /tmp/go.tar.gz step).
🧹 Nitpick comments (6)
hack/update-go-pins.sh (1)
27-28: Fail fast if pin replacement does not apply.These in-place regex updates can silently no-op if file formatting changes. Add post-update checks so the script exits non-zero when expected pins are not written.
Suggested patch
perl -0pi -e 's/^ARG GO_VERSION=.*/ARG GO_VERSION='"${version}"'/m; s/^ARG GO_SHA256_AMD64=.*/ARG GO_SHA256_AMD64='"${sha_amd64}"'/m; s/^ARG GO_SHA256_ARM64=.*/ARG GO_SHA256_ARM64='"${sha_arm64}"'/m' "${CONTAINERFILE}" perl -0pi -e 's/^ENV GO_VERSION=.*/ENV GO_VERSION='"${version}"'/m; s/^ENV GO_SHA256_AMD64=.*/ENV GO_SHA256_AMD64='"${sha_amd64}"'/m; s/^ENV GO_SHA256_ARM64=.*/ENV GO_SHA256_ARM64='"${sha_arm64}"'/m' "${BUILD_ROOT_DOCKERFILE}" + +grep -q "^ARG GO_VERSION=${version}$" "${CONTAINERFILE}" || { echo "Failed to update GO_VERSION in ${CONTAINERFILE}" >&2; exit 1; } +grep -q "^ARG GO_SHA256_AMD64=${sha_amd64}$" "${CONTAINERFILE}" || { echo "Failed to update GO_SHA256_AMD64 in ${CONTAINERFILE}" >&2; exit 1; } +grep -q "^ARG GO_SHA256_ARM64=${sha_arm64}$" "${CONTAINERFILE}" || { echo "Failed to update GO_SHA256_ARM64 in ${CONTAINERFILE}" >&2; exit 1; } +grep -q "^ENV GO_VERSION=${version}$" "${BUILD_ROOT_DOCKERFILE}" || { echo "Failed to update GO_VERSION in ${BUILD_ROOT_DOCKERFILE}" >&2; exit 1; } +grep -q "^ENV GO_SHA256_AMD64=${sha_amd64}$" "${BUILD_ROOT_DOCKERFILE}" || { echo "Failed to update GO_SHA256_AMD64 in ${BUILD_ROOT_DOCKERFILE}" >&2; exit 1; } +grep -q "^ENV GO_SHA256_ARM64=${sha_arm64}$" "${BUILD_ROOT_DOCKERFILE}" || { echo "Failed to update GO_SHA256_ARM64 in ${BUILD_ROOT_DOCKERFILE}" >&2; exit 1; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/update-go-pins.sh` around lines 27 - 28, After running the in-place perl replacements, verify the replacements actually applied and fail fast if not: for each target file (CONTAINERFILE and BUILD_ROOT_DOCKERFILE) check that the updated ARG/ENV lines containing the variables version, sha_amd64 and sha_arm64 are present (e.g. search for "ARG GO_VERSION=${version}" and corresponding GO_SHA256_* entries in CONTAINERFILE, and "ENV GO_VERSION=${version}" and GO_SHA256_* entries in BUILD_ROOT_DOCKERFILE); if any expected line is missing, print a clear error and exit non-zero so the script fails rather than silently no-op. Ensure checks reference the same variable names (version, sha_amd64, sha_arm64) and run immediately after the respective perl commands that modify each file.rebasebot/lifecycle_hooks.py (2)
55-59: Consider using!rconversion flag for cleaner repr formatting.The Ruff linter suggests using explicit conversion flags instead of calling
repr()inside f-strings.Proposed fix
def __repr__(self): return ( f"LifecycleHookScriptResult(return_code={self.return_code}, " - f"stdout={repr(self.stdout)}, stderr={repr(self.stderr)})" + f"stdout={self.stdout!r}, stderr={self.stderr!r})" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rebasebot/lifecycle_hooks.py` around lines 55 - 59, The __repr__ in class LifecycleHookScriptResult currently calls repr() inside the f-strings; change it to use the !r conversion flag for cleaner formatting by updating the f-string in __repr__ to use {self.stdout!r} and {self.stderr!r} (and keep {self.return_code} as-is) so the representation uses built-in conversion instead of explicit repr() calls.
243-244: Use explicitstr | Nonefor parameter withNonedefault.PEP 484 prohibits implicit
Optional. Sinceref_filterdefaults toNone, its type annotation should reflect that.Proposed fix
-def _fetch_branch(gitwd: git.Repo, remote: str, branch: str, ref_filter: str = None): +def _fetch_branch(gitwd: git.Repo, remote: str, branch: str, ref_filter: str | None = None): return gitwd.git.fetch(remote, branch, filter=ref_filter)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rebasebot/lifecycle_hooks.py` around lines 243 - 244, Update the _fetch_branch function signature to use an explicit nullable type for ref_filter (e.g., change its annotation to "str | None" or "Optional[str]") so the default None is properly typed; adjust any related type hints or imports (typing.Optional) if you choose Optional[str], and ensure the parameter is still passed unchanged to gitwd.git.fetch within _fetch_branch.Makefile (1)
20-22: Consider adding atestalias for discoverability.Many developers expect
make testto run tests. Adding an alias would improve usability.Proposed addition
.PHONY: unittests unittests: ## Run unit & integration tests PYTHON_BIN=$(PYTHON_BIN) hack/tests.sh + +.PHONY: test +test: unittests ## Alias for unittests🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 20 - 22, Add a discoverable alias target named "test" that invokes the existing "unittests" target so running "make test" runs the same test flow; update the Makefile to add a PHONY for test (alongside unittests) and create a `test:` target that depends on or calls `unittests` to avoid duplicating the test command.rebasebot/cli.py (1)
342-345: Consider removing redundantstr()calls in f-strings.F-strings automatically call
__str__()on objects, makingstr(e)redundant.Proposed fix
logging.error( - f"Error running source repo hook: {str(e)}", + f"Error running source repo hook: {e}", exc_info=True, ) # Log the full stack trace @@ logging.error( - f"Error occurred while initializing lifecycle hooks: {str(e)}", + f"Error occurred while initializing lifecycle hooks: {e}", exc_info=True, )Also applies to: 351-354
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rebasebot/cli.py` around lines 342 - 345, Remove the redundant str() calls inside the f-strings used in the logging.error calls (the ones that currently do f"Error running source repo hook: {str(e)}" and the similar call at the later logging.error around lines 351-354); replace {str(e)} with {e} (or simply {e!s}) so f-strings rely on the object's __str__ automatically while keeping exc_info=True unchanged and targeting the existing logging.error calls and exception variable e.rebasebot/bot.py (1)
354-354: Use explicitset | Nonefor parameter withNonedefault.PEP 484 prohibits implicit
Optional. Sinceonly_filesdefaults toNone, its type annotation should reflect that.Proposed fix
-def _check_upstream_content_loss(gitwd: git.Repo, source_branch: str, only_files: set = None) -> list: +def _check_upstream_content_loss(gitwd: git.Repo, source_branch: str, only_files: set | None = None) -> list:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rebasebot/bot.py` at line 354, Update the _check_upstream_content_loss function signature to explicitly annotate the nullable parameter by changing only_files: set = None to only_files: set | None = None (or Optional[set] if your codebase prefers typing.Optional), and adjust any internal references if type-specific operations require a concrete generic (e.g., cast to set or check for None before iterating). This makes the parameter's optional nature explicit per PEP 484/604 and avoids implicit Optional typing for only_files in _check_upstream_content_loss.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.openshift-ci/Dockerfile.build_root:
- Around line 11-18: The RUN layer that installs packages and Go (the block
starting with "RUN dnf install -y tar gzip git make && \\" and the "python -m
pip install uv" invocation) leaves DNF and pip caches in the image; update that
same RUN chain to run "dnf clean all" and remove DNF caches (e.g., rm -rf
/var/cache/dnf /var/cache/yum /var/run/dnf*) after installs, and install Python
packages with pip using --no-cache-dir (or run "python -m pip cache purge" / rm
-rf ~/.cache/pip) so all package manager caches are deleted in the same layer
where GO download and tar extraction occur (keep the existing rm -f
/tmp/go.tar.gz step).
---
Nitpick comments:
In `@hack/update-go-pins.sh`:
- Around line 27-28: After running the in-place perl replacements, verify the
replacements actually applied and fail fast if not: for each target file
(CONTAINERFILE and BUILD_ROOT_DOCKERFILE) check that the updated ARG/ENV lines
containing the variables version, sha_amd64 and sha_arm64 are present (e.g.
search for "ARG GO_VERSION=${version}" and corresponding GO_SHA256_* entries in
CONTAINERFILE, and "ENV GO_VERSION=${version}" and GO_SHA256_* entries in
BUILD_ROOT_DOCKERFILE); if any expected line is missing, print a clear error and
exit non-zero so the script fails rather than silently no-op. Ensure checks
reference the same variable names (version, sha_amd64, sha_arm64) and run
immediately after the respective perl commands that modify each file.
In `@Makefile`:
- Around line 20-22: Add a discoverable alias target named "test" that invokes
the existing "unittests" target so running "make test" runs the same test flow;
update the Makefile to add a PHONY for test (alongside unittests) and create a
`test:` target that depends on or calls `unittests` to avoid duplicating the
test command.
In `@rebasebot/bot.py`:
- Line 354: Update the _check_upstream_content_loss function signature to
explicitly annotate the nullable parameter by changing only_files: set = None to
only_files: set | None = None (or Optional[set] if your codebase prefers
typing.Optional), and adjust any internal references if type-specific operations
require a concrete generic (e.g., cast to set or check for None before
iterating). This makes the parameter's optional nature explicit per PEP 484/604
and avoids implicit Optional typing for only_files in
_check_upstream_content_loss.
In `@rebasebot/cli.py`:
- Around line 342-345: Remove the redundant str() calls inside the f-strings
used in the logging.error calls (the ones that currently do f"Error running
source repo hook: {str(e)}" and the similar call at the later logging.error
around lines 351-354); replace {str(e)} with {e} (or simply {e!s}) so f-strings
rely on the object's __str__ automatically while keeping exc_info=True unchanged
and targeting the existing logging.error calls and exception variable e.
In `@rebasebot/lifecycle_hooks.py`:
- Around line 55-59: The __repr__ in class LifecycleHookScriptResult currently
calls repr() inside the f-strings; change it to use the !r conversion flag for
cleaner formatting by updating the f-string in __repr__ to use {self.stdout!r}
and {self.stderr!r} (and keep {self.return_code} as-is) so the representation
uses built-in conversion instead of explicit repr() calls.
- Around line 243-244: Update the _fetch_branch function signature to use an
explicit nullable type for ref_filter (e.g., change its annotation to "str |
None" or "Optional[str]") so the default None is properly typed; adjust any
related type hints or imports (typing.Optional) if you choose Optional[str], and
ensure the parameter is still passed unchanged to gitwd.git.fetch within
_fetch_branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f6197ea-4eb1-4f6f-aa8f-a1e139dd3c6e
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.openshift-ci/Dockerfile.build_root.pylintrcAGENTS.mdContainerfileMakefileREADME.mdhack/configure-hooks.shhack/linthack/tests.shhack/update-go-pins.shpyproject.tomlrebasebot/bot.pyrebasebot/cli.pyrebasebot/github.pyrebasebot/lifecycle_hooks.pyrequirements-hacking.txtrequirements.txtsetup.cfgsetup.pytests/conftest.pytests/test_bot.pytests/test_cli.pytests/test_conflict_policy.pytests/test_rebases.py
💤 Files with no reviewable changes (7)
- requirements-hacking.txt
- requirements.txt
- hack/configure-hooks.sh
- hack/lint
- .pylintrc
- setup.cfg
- setup.py
✅ Files skipped from review due to trivial changes (3)
- tests/test_conflict_policy.py
- README.md
- tests/test_rebases.py
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/test_bot.py
- pyproject.toml
- tests/conftest.py
- rebasebot/github.py
RadekManak
force-pushed
the
overhaul-tooling
branch
from
b12933b to
08e48d7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Makefile (1)
54-68: Consider edge case indepstarget.The
depstarget has good CI/local separation. However, if someone runsmake depslocally without first runningmake venv, thevenvdependency will create the environment, but the subsequent pip install of uv might fail ifenv/bin/pythondoesn't exist yet (race condition unlikely but possible if venv creation fails silently).Also note: in CI mode (line 68),
--no-install-projectis used which won't install therebasebotpackage itself. This is intentional for the read-only checkout, but worth documenting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 54 - 68, The deps target can proceed before a usable virtualenv exists (venv may fail silently), so modify deps (and its commands that reference env) to explicitly verify the venv was created and abort or create it if missing: after calling the venv dependency, check for the venv python binary (e.g. env/bin/python or $(CURDIR)/env/bin/python) and exit with a clear error if absent (or run python -m venv env again) before running pip/uv; update the Windows/CI branch accordingly and add a short comment near the deps and venv targets documenting that CI uses --no-install-project intentionally for the read‑only checkout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Makefile`:
- Around line 1-4: The IS_CI detection is wrong: replace the VIRTUAL_ENV check
that sets IS_CI with a check for OPENSHIFT_CI (i.e. set IS_CI := true when
OPENSHIFT_CI == "true") so CI runs see IS_CI=true; update any logic that depends
on IS_CI (targets lint, build, test, deps) so they use the correct PYTHON_BIN
and avoid creating env/ in read-only CI, and add the required --no-cache flag to
RUFF_ARGS when IS_CI is true; touch references: the Makefile variable IS_CI, the
VIRTUAL_ENV check, RUFF_ARGS, and targets lint/build/test/deps (and ensure
hack/tests.sh usage of OPENSHIFT_CI remains consistent).
In `@rebasebot/lifecycle_hooks.py`:
- Around line 86-88: Change the file-permission mask used when writing hook
scripts to be owner-only; replace os.chmod(script_file_path, 0o755) with
os.chmod(script_file_path, 0o700) for the write block that opens
script_file_path and for the similar chmod call around lines 135-140 so fetched
hook scripts are executable only by the owner (refer to the with
open(script_file_path, "w", encoding="latin1") as f: block and the os.chmod
calls).
- Around line 169-170: The regex assigned to self.script_location currently uses
the branch capture group ([^/]*?) which rejects branch names containing slashes;
update that group to allow slash characters and stop at the colon (for example
use ([^:]+?) or (.+?) instead of ([^/]*?)) so branch names like "feature/foo"
are accepted; modify the pattern string
"^git:(https://([^/]+)/([^/]+)/([^/]+))/([^/]*?):(.*)$" accordingly in
lifecycle_hooks.py where self.script_location is set.
---
Nitpick comments:
In `@Makefile`:
- Around line 54-68: The deps target can proceed before a usable virtualenv
exists (venv may fail silently), so modify deps (and its commands that reference
env) to explicitly verify the venv was created and abort or create it if
missing: after calling the venv dependency, check for the venv python binary
(e.g. env/bin/python or $(CURDIR)/env/bin/python) and exit with a clear error if
absent (or run python -m venv env again) before running pip/uv; update the
Windows/CI branch accordingly and add a short comment near the deps and venv
targets documenting that CI uses --no-install-project intentionally for the
read‑only checkout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b0454221-b350-47d9-a2c1-b8f47b8c4469
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.openshift-ci/Dockerfile.build_root.pylintrcAGENTS.mdContainerfileMakefileREADME.mdhack/configure-hooks.shhack/linthack/tests.shhack/update-go-pins.shpyproject.tomlrebasebot/bot.pyrebasebot/cli.pyrebasebot/github.pyrebasebot/lifecycle_hooks.pyrequirements-hacking.txtrequirements.txtsetup.cfgsetup.pytests/conftest.pytests/test_bot.pytests/test_cli.pytests/test_conflict_policy.pytests/test_rebases.py
💤 Files with no reviewable changes (7)
- hack/configure-hooks.sh
- requirements-hacking.txt
- hack/lint
- requirements.txt
- .pylintrc
- setup.cfg
- setup.py
✅ Files skipped from review due to trivial changes (4)
- README.md
- .openshift-ci/Dockerfile.build_root
- tests/test_rebases.py
- tests/test_conflict_policy.py
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/test_bot.py
- hack/update-go-pins.sh
- Containerfile
- rebasebot/github.py
- rebasebot/cli.py
- rebasebot/bot.py
RadekManak
force-pushed
the
overhaul-tooling
branch
from
08e48d7 to
8d4f126
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hack/tests.sh (1)
24-42: Use an args array instead of unquoted string expansion for pytest arguments.The unquoted
$PYTEST_ARGSon line 42 triggers ShellCheck's SC2086 warning. Refactor to build arguments in an array instead of concatenating strings, then pass"${PYTEST_ARGS_ARRAY[@]}"to pytest.Proposed refactor
PYTHON_BIN=${PYTHON_BIN:-python} -PYTEST_ARGS=${PYTEST_ARGS:-"-vv --cov=rebasebot"} +PYTEST_ARGS=${PYTEST_ARGS:-"-vv --cov=rebasebot"} +read -r -a PYTEST_ARGS_ARRAY <<< "$PYTEST_ARGS" @@ - PYTEST_ARGS="${PYTEST_ARGS} -p no:cacheprovider --cov-report=term --cov-report=html:${ARTIFACT_DIR}/cov-report --junitxml=${ARTIFACT_DIR}/junit_rebasebot_tests.xml" + PYTEST_ARGS_ARRAY+=( + -p no:cacheprovider + --cov-report=term + "--cov-report=html:${ARTIFACT_DIR}/cov-report" + "--junitxml=${ARTIFACT_DIR}/junit_rebasebot_tests.xml" + ) fi set -x -"$PYTHON_BIN" -m pytest $PYTEST_ARGS "$REPO_ROOT" +"$PYTHON_BIN" -m pytest "${PYTEST_ARGS_ARRAY[@]}" "$REPO_ROOT"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/tests.sh` around lines 24 - 42, The script currently expands PYTEST_ARGS unquoted (variable PYTEST_ARGS and the final pytest invocation) causing SC2086; change to use a bash array (e.g., PYTEST_ARGS_ARRAY) instead of a space-joined string: initialize PYTEST_ARGS_ARRAY from default PYTEST_ARGS split into array form, in the CI block append additional options to PYTEST_ARGS_ARRAY (do not concatenate into a string), and replace the final invocation "$PYTHON_BIN" -m pytest $PYTEST_ARGS "$REPO_ROOT" with "$PYTHON_BIN" -m pytest "${PYTEST_ARGS_ARRAY[@]}" "$REPO_ROOT"; ensure all references to PYTEST_ARGS that add flags (inside the CI conditional) push into the array rather than modifying a plain string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hack/tests.sh`:
- Around line 24-42: The script currently expands PYTEST_ARGS unquoted (variable
PYTEST_ARGS and the final pytest invocation) causing SC2086; change to use a
bash array (e.g., PYTEST_ARGS_ARRAY) instead of a space-joined string:
initialize PYTEST_ARGS_ARRAY from default PYTEST_ARGS split into array form, in
the CI block append additional options to PYTEST_ARGS_ARRAY (do not concatenate
into a string), and replace the final invocation "$PYTHON_BIN" -m pytest
$PYTEST_ARGS "$REPO_ROOT" with "$PYTHON_BIN" -m pytest "${PYTEST_ARGS_ARRAY[@]}"
"$REPO_ROOT"; ensure all references to PYTEST_ARGS that add flags (inside the CI
conditional) push into the array rather than modifying a plain string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 2900f366-1e81-412f-a338-a58a655b0131
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.openshift-ci/Dockerfile.build_root.pylintrcAGENTS.mdContainerfileMakefileREADME.mdhack/configure-hooks.shhack/linthack/tests.shhack/update-go-pins.shpyproject.tomlrebasebot/bot.pyrebasebot/cli.pyrebasebot/github.pyrebasebot/lifecycle_hooks.pyrequirements-hacking.txtrequirements.txtsetup.cfgsetup.pytests/conftest.pytests/test_bot.pytests/test_cli.pytests/test_conflict_policy.pytests/test_rebases.py
💤 Files with no reviewable changes (7)
- requirements-hacking.txt
- hack/configure-hooks.sh
- requirements.txt
- setup.cfg
- hack/lint
- .pylintrc
- setup.py
✅ Files skipped from review due to trivial changes (6)
- tests/test_conflict_policy.py
- README.md
- tests/test_rebases.py
- hack/update-go-pins.sh
- rebasebot/cli.py
- .openshift-ci/Dockerfile.build_root
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/test_bot.py
- Containerfile
- tests/conftest.py
- rebasebot/github.py
damdo
reviewed
Apr 22, 2026
Contributor
damdo
left a comment
damdo
left a comment
There was a problem hiding this comment.
LGTM
aside from the SHA part, which looks quite hairy and complex. I don't think we need that level of SHA matching, we should be happy with just the version.
Comment on lines
+1
to
+32
| #!/bin/bash | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| REPO_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. | ||
| CONTAINERFILE="${REPO_ROOT}/Containerfile" | ||
| BUILD_ROOT_DOCKERFILE="${REPO_ROOT}/.openshift-ci/Dockerfile.build_root" | ||
| RELEASES_URL="https://go.dev/dl/?mode=json&include=all" | ||
|
|
||
| command -v curl >/dev/null | ||
| command -v jq >/dev/null | ||
| command -v perl >/dev/null | ||
|
|
||
| version="${1:-$(sed -n 's/^ARG GO_VERSION=//p' "${CONTAINERFILE}")}" | ||
| version="${version#go}" | ||
| release_version="go${version}" | ||
|
|
||
| release_json="$(curl -fsSL "${RELEASES_URL}")" | ||
| sha_amd64="$(printf '%s\n' "${release_json}" | jq -r --arg version "${release_version}" '.[] | select(.version == $version) | .files[] | select(.kind == "archive" and .os == "linux" and .arch == "amd64") | .sha256')" | ||
| sha_arm64="$(printf '%s\n' "${release_json}" | jq -r --arg version "${release_version}" '.[] | select(.version == $version) | .files[] | select(.kind == "archive" and .os == "linux" and .arch == "arm64") | .sha256')" | ||
|
|
||
| if [[ -z "${sha_amd64}" || "${sha_amd64}" == "null" || -z "${sha_arm64}" || "${sha_arm64}" == "null" ]]; then | ||
| echo "Could not find linux archive checksums for ${release_version}" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| perl -0pi -e 's/^ARG GO_VERSION=.*/ARG GO_VERSION='"${version}"'/m; s/^ARG GO_SHA256_AMD64=.*/ARG GO_SHA256_AMD64='"${sha_amd64}"'/m; s/^ARG GO_SHA256_ARM64=.*/ARG GO_SHA256_ARM64='"${sha_arm64}"'/m' "${CONTAINERFILE}" | ||
| perl -0pi -e 's/^ENV GO_VERSION=.*/ENV GO_VERSION='"${version}"'/m; s/^ENV GO_SHA256_AMD64=.*/ENV GO_SHA256_AMD64='"${sha_amd64}"'/m; s/^ENV GO_SHA256_ARM64=.*/ENV GO_SHA256_ARM64='"${sha_arm64}"'/m' "${BUILD_ROOT_DOCKERFILE}" | ||
|
|
||
| echo "Updated Go pins to ${version}" | ||
| echo " amd64: ${sha_amd64}" | ||
| echo " arm64: ${sha_arm64}" |
Contributor
There was a problem hiding this comment.
I'd reduce this to just a check that verifies the GO VERSION matches between the two.
Comment on lines
+7
to
+20
| ENV GO_SHA256_AMD64=00859d7bd6defe8bf84d9db9e57b9a4467b2887c18cd93ae7460e713db774bc1 | ||
| ENV GO_SHA256_ARM64=ec342e7389b7f489564ed5463c63b16cf8040023dabc7861256677165a8c0e2b | ||
| ENV PATH="/usr/local/go/bin:$PATH" | ||
|
|
||
| RUN dnf install -y tar gzip git make && \ | ||
| ARCH=$(case "${TARGETARCH:-amd64}" in aarch64) echo "arm64" ;; amd64) echo "amd64" ;; *) echo "${TARGETARCH:-amd64}" ;; esac) && \ | ||
| GO_SHA256=$(case "${ARCH}" in amd64) echo "${GO_SHA256_AMD64}" ;; arm64) echo "${GO_SHA256_ARM64}" ;; *) echo "Unsupported TARGETARCH: ${TARGETARCH:-amd64}" >&2; exit 1 ;; esac) && \ | ||
| curl -fLsS -o /tmp/go.tar.gz "https://go.dev/dl/go${GO_VERSION}.linux-${ARCH}.tar.gz" && \ | ||
| echo "${GO_SHA256} /tmp/go.tar.gz" | sha256sum -c - && \ | ||
| tar -C /usr/local -xzf /tmp/go.tar.gz go/ && \ | ||
| rm -f /tmp/go.tar.gz && \ | ||
| python -m pip install --no-cache-dir uv && \ | ||
| dnf clean all && \ | ||
| rm -rf /var/cache/dnf /var/cache/yum /var/run/dnf* |
RadekManak
force-pushed
the
overhaul-tooling
branch
from
8d4f126 to
74cd61c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
hack/update-go-pins.sh (1)
5-7: Use an absoluteREPO_ROOTto avoid cwd-dependent behavior.Current path derivation is relative and can break when invoked from outside the repository root.
Proposed fix
-REPO_ROOT=$(dirname "${BASH_SOURCE[0]}")/.. +REPO_ROOT="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" CONTAINERFILE="${REPO_ROOT}/Containerfile" BUILD_ROOT_DOCKERFILE="${REPO_ROOT}/.openshift-ci/Dockerfile.build_root"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/update-go-pins.sh` around lines 5 - 7, REPO_ROOT is set using a relative path which makes script behavior cwd-dependent; change the REPO_ROOT assignment in hack/update-go-pins.sh to resolve to an absolute path by taking the script directory (dirname "${BASH_SOURCE[0]}"), moving up one level, and using a reliable pwd resolution (e.g., cd into that path and capture the physical absolute path) so CONTAINERFILE and BUILD_ROOT_DOCKERFILE are always derived from an absolute REPO_ROOT.hack/tests.sh (1)
26-42: ConvertPYTEST_ARGSto an array to prevent word-splitting issues.Line 42 expands
$PYTEST_ARGSunquoted, triggering shellcheck's SC2086 warning. The array-based approach below safely preserves argument boundaries while allowing the CI branch to append additional options.Proposed diff
-PYTEST_ARGS=${PYTEST_ARGS:-"-vv --cov=rebasebot"} +PYTEST_ARGS=${PYTEST_ARGS:-"-vv --cov=rebasebot"} +read -r -a PYTEST_ARGS_ARR <<<"$PYTEST_ARGS" @@ - PYTEST_ARGS="${PYTEST_ARGS} -p no:cacheprovider --cov-report=term --cov-report=html:${ARTIFACT_DIR}/cov-report --junitxml=${ARTIFACT_DIR}/junit_rebasebot_tests.xml" + PYTEST_ARGS_ARR+=( + -p no:cacheprovider + --cov-report=term + --cov-report=html:${ARTIFACT_DIR}/cov-report + --junitxml=${ARTIFACT_DIR}/junit_rebasebot_tests.xml + ) fi @@ -"$PYTHON_BIN" -m pytest $PYTEST_ARGS "$REPO_ROOT" +"$PYTHON_BIN" -m pytest "${PYTEST_ARGS_ARR[@]}" "$REPO_ROOT"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/tests.sh` around lines 26 - 42, PYTEST_ARGS is currently a string that gets expanded unquoted causing word-splitting (SC2086); change it to a shell array and update how it's set, appended, and invoked: initialize with an array (e.g. read -r -a PYTEST_ARGS <<< "${PYTEST_ARGS:-'-vv --cov=rebasebot'}" or PYTEST_ARGS=(-vv --cov=rebasebot)), in the CI branch append options with array syntax (PYTEST_ARGS+=( -p no:cacheprovider --cov-report=term --cov-report=html:"${ARTIFACT_DIR}/cov-report" --junitxml="${ARTIFACT_DIR}/junit_rebasebot_tests.xml" )), and call pytest with the array expansion ("${PYTEST_ARGS[@]}") in the final command instead of unquoted $PYTEST_ARGS so argument boundaries are preserved.rebasebot/lifecycle_hooks.py (1)
325-329: Alignfetch_hook_scriptstype hints with runtime behavior.Line 325 annotates
github_app_provideras required, butLifecycleHookScript.fetch_script()acceptsNoneand non-GitHub/local flows can work without it. PreferGithubAppProvider | Nonefor accurate typing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rebasebot/lifecycle_hooks.py` around lines 325 - 329, The type hint for the parameter github_app_provider on fetch_hook_scripts is inaccurate: update the signature of fetch_hook_scripts to accept GithubAppProvider | None (or Optional[GithubAppProvider]) to match LifecycleHookScript.fetch_script which accepts None and to reflect non-GitHub/local flows; ensure usages inside fetch_hook_scripts pass the possibly-None value to script.fetch_script(temp_hook_dir=self.tmp_hook_scripts_dir, gitwd=gitwd, github=github_app_provider) without changing runtime logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/update-go-pins.sh`:
- Around line 11-12: The script computes version from CONTAINERFILE into the
variable version and then strips a "go" prefix, but it never checks for an empty
result; add a fail-fast check after deriving and normalizing version (after the
lines that set version="${1:-$(sed -n 's/^ARG GO_VERSION=//p'
"${CONTAINERFILE}")}" and version="${version#go}"): if version is empty (test -z
"$version") print a clear error to stderr referencing CONTAINERFILE/GO_VERSION
and exit non‑zero so the script does not proceed to rewrite files with a blank
value.
- Around line 17-23: The script currently only checks that container_go_version
and build_root_go_version match each other which can pass when both are empty;
update the validation to also assert a requested/goal version: read a
requested_go_version (prefer an explicit first script argument or fallback to
REQUESTED_GO_VERSION env var), ensure requested_go_version is non-empty, then
verify container_go_version == requested_go_version AND build_root_go_version ==
requested_go_version (use the existing variable names container_go_version and
build_root_go_version for the comparisons) and fail with a clear error if any
check does not hold.
In `@rebasebot/lifecycle_hooks.py`:
- Around line 165-170: The code calls
_extract_script_details(self.script_location, temp_hook_dir) before validating
the script format, which can cause unpack/split errors for malformed "git:"
values; fix by first matching/validating self.script_location against the
expected patterns (the remote_git_pattern_match and local_git_pattern_match
regexes) and only then call _extract_script_details if a pattern matched,
otherwise raise the intended validation error; update both occurrences around
the lines using remote_git_pattern_match/local_git_pattern_match to perform
validation prior to extracting git_ref, file_path, and self.script_file_path.
---
Nitpick comments:
In `@hack/tests.sh`:
- Around line 26-42: PYTEST_ARGS is currently a string that gets expanded
unquoted causing word-splitting (SC2086); change it to a shell array and update
how it's set, appended, and invoked: initialize with an array (e.g. read -r -a
PYTEST_ARGS <<< "${PYTEST_ARGS:-'-vv --cov=rebasebot'}" or PYTEST_ARGS=(-vv
--cov=rebasebot)), in the CI branch append options with array syntax
(PYTEST_ARGS+=( -p no:cacheprovider --cov-report=term
--cov-report=html:"${ARTIFACT_DIR}/cov-report"
--junitxml="${ARTIFACT_DIR}/junit_rebasebot_tests.xml" )), and call pytest with
the array expansion ("${PYTEST_ARGS[@]}") in the final command instead of
unquoted $PYTEST_ARGS so argument boundaries are preserved.
In `@hack/update-go-pins.sh`:
- Around line 5-7: REPO_ROOT is set using a relative path which makes script
behavior cwd-dependent; change the REPO_ROOT assignment in
hack/update-go-pins.sh to resolve to an absolute path by taking the script
directory (dirname "${BASH_SOURCE[0]}"), moving up one level, and using a
reliable pwd resolution (e.g., cd into that path and capture the physical
absolute path) so CONTAINERFILE and BUILD_ROOT_DOCKERFILE are always derived
from an absolute REPO_ROOT.
In `@rebasebot/lifecycle_hooks.py`:
- Around line 325-329: The type hint for the parameter github_app_provider on
fetch_hook_scripts is inaccurate: update the signature of fetch_hook_scripts to
accept GithubAppProvider | None (or Optional[GithubAppProvider]) to match
LifecycleHookScript.fetch_script which accepts None and to reflect
non-GitHub/local flows; ensure usages inside fetch_hook_scripts pass the
possibly-None value to
script.fetch_script(temp_hook_dir=self.tmp_hook_scripts_dir, gitwd=gitwd,
github=github_app_provider) without changing runtime logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3bf6194d-5fba-49ae-8ac4-c41351ad8f60
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.openshift-ci/Dockerfile.build_root.pylintrcAGENTS.mdContainerfileMakefileREADME.mdhack/configure-hooks.shhack/linthack/tests.shhack/update-go-pins.shpyproject.tomlrebasebot/bot.pyrebasebot/cli.pyrebasebot/github.pyrebasebot/lifecycle_hooks.pyrequirements-hacking.txtrequirements.txtsetup.cfgsetup.pytests/conftest.pytests/test_bot.pytests/test_cli.pytests/test_conflict_policy.pytests/test_rebases.py
💤 Files with no reviewable changes (7)
- hack/configure-hooks.sh
- requirements.txt
- setup.py
- hack/lint
- .pylintrc
- setup.cfg
- requirements-hacking.txt
✅ Files skipped from review due to trivial changes (5)
- README.md
- .openshift-ci/Dockerfile.build_root
- tests/test_conflict_policy.py
- rebasebot/cli.py
- tests/test_rebases.py
🚧 Files skipped from review as they are similar to previous changes (4)
- Containerfile
- rebasebot/bot.py
- rebasebot/github.py
- tests/test_bot.py
RadekManak
force-pushed
the
overhaul-tooling
branch
from
74cd61c to
ecaa0b7
Compare
RadekManak
force-pushed
the
overhaul-tooling
branch
from
ecaa0b7 to
ab5fea4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pyproject.toml+uv.lock, remove legacysetup.py/setup.cfg/requirements*.txt/.pylintrctooling, and refresh theMakefile/ test scripts around theenvvenv workflow and Ruff-based linting.openshift-ci/Dockerfile.build_rootinto this repo so CI image changes can be made and reviewed alongside the code that depends on them, and harden both CI/runtime container definitions by pinning Go1.25.9with SHA256 verification foramd64andarm64hack/update-go-pins.shto keep the pinned Go version and hashes in sync across both container definitions, and update docs/tests for the new dev workflow plus hook/conflict-policy coverage already exercised on this branch