Repository navigation
Conversation
Signed-off-by: Vignesh <vickyofficial26@gmail.com>
|
|
panthera-momagdii
left a comment
There was a problem hiding this comment.
Direction is good, but H-CI-1 is not complete yet against the work-package acceptance you shared.
Please close these before approval:
- The new image workflow only runs on
pushtomain/ manual dispatch, so this PR has no hosted Docker build validation. Add a PR build-only path (no GHCR push) so a broken Dockerfile cannot merge green. - After the publish path is proven, record/pin the produced image digest in the manifest.
- Use that same pinned image for contributor development through a devcontainer, and show a contributor can open it and reach a working build.
- Add evidence for the cold pull-request software gate staying within the 10-minute budget.
- The image currently installs
ros-jazzy-rplidar-ros, while the current 2.0 S3 path is moving tosllidar_ros2from the pinned source manifest. Please either document why the legacy package is still intentionally needed in the baseline image or align the image with the current driver path.
Repository-quality and CLA are green, but they do not validate the container build/publish path itself. Re-request review once the hosted image build and H-CI-1 acceptance evidence are present.
panthera-momagdii
left a comment
There was a problem hiding this comment.
Direction is good, but H-CI-1 is not complete yet against the stated acceptance criteria.
Please close these before approval:
- The image workflow only runs on push to
main/ manual dispatch, so this PR has no hosted Docker build validation. Add a pull-request build-only path (no GHCR push) so a broken Dockerfile cannot merge green. - After the publish path is proven, record and pin the produced image digest in the manifest.
- Use the same pinned image for contributor development through a devcontainer and show that a contributor can open it and reach a working build.
- Add evidence that a cold pull-request software gate stays within the 10-minute budget.
- The image currently installs
ros-jazzy-rplidar-ros, while the current 2.0 S3 path is moving tosllidar_ros2from the pinned source manifest. Either document why the legacy package is intentionally kept in the baseline image or align the image with the current driver path.
Repository-quality and CLA are green, but they do not validate the container build/publish path itself. Re-request review once the hosted image build and H-CI-1 acceptance evidence are present.
Signed-off-by: Arumuga Vignesh S R <129583467+wikki26@users.noreply.github.com>
Signed-off-by: Arumuga Vignesh S R <129583467+wikki26@users.noreply.github.com>
a548857 to
d929043
Compare
Signed-off-by: Arumuga Vignesh S R <129583467+wikki26@users.noreply.github.com>
Signed-off-by: Arumuga Vignesh S R <129583467+wikki26@users.noreply.github.com>
Signed-off-by: Arumuga Vignesh S R <129583467+wikki26@users.noreply.github.com>
Signed-off-by: Arumuga Vignesh S R <129583467+wikki26@users.noreply.github.com>
|
I've pushed the latest H-CI-1 updates and wanted to share the validation status against your review feedback. Completed:
Validation:
Still pending:
Could you review the latest changes and let me know if any further adjustments are needed for H-CI-1 acceptance? Thanks! |
panthera-momagdii
left a comment
There was a problem hiding this comment.
Reviewed the current two-file image/workflow change at a1f619307557ad3a0680065127df7e027d47dd83.
Review basis: openAMRobot/.github, branch h-ci-1-baseline-image, head a1f6193, base 539da19. Expected outcome: acknowledge the image-build follow-up and distinguish the evidence now present from the remaining H-CI-1 acceptance work.
The current change addresses the PR-safe build path, immutable ROS base-image pin, and removal of the legacy ros-jazzy-rplidar-ros package from this image. PR events build without logging into or pushing to GHCR; publishing is separated into the intended main/manual-candidate paths. Action references are pinned. The current image-build run passed.
The candidate is also pinned in manifest #13, and platform-sw #57 demonstrates that it can be pulled and build the seven packages. That is useful integration evidence, but #57's subsequent test failure remains open; the successful build does not make its full gate pass.
The remaining H-CI-1 evidence is still:
- a successful run of the automated GHCR publication path, with the resulting digest;
- an actual VS Code devcontainer startup using that pinned image;
- the complete passing cold gate with its measured duration against the agreed under-10-minute target.
Please attach those run/startup records and keep the PR description aligned with the evidence available. The S3 sllidar integration remains a separate follow-up as documented.
No new blocking source finding in this image-build delta. This follow-up acknowledges the progress; the existing H-CI-1 acceptance review remains incomplete until the outstanding evidence is supplied.
ChatGPT-assisted source and hosted-evidence review; no local devcontainer startup or hardware execution.
BotshareAI
left a comment
There was a problem hiding this comment.
Good: digest-pinned base, actions pinned by SHA, a PR build without push, legacy rplidar package removed. Before merge:
Split the jobs: a build job with contents: read for pull_request, and a publish job with packages: write only for push to main or workflow_dispatch.
The publish condition hardcodes refs/heads/h-ci-1-baseline-image. That branch disappears after merge. Allow dispatch from main only.
Add ros-jazzy-twist-mux (command arbitration, ui #31). Remove ros-jazzy-camera-ros unless a package depends on it; the base camera is the Gemini 336L on the Orbbec driver.
Keep rmw_cyclonedds_cpp as default; it will be recorded as the project RMW in the register.
Provenance: the digest pinned in manifest #13 and platform-sw #57 (sha256:1653d4…) was published from the feature branch. After merge, publish from main, record the run ID and digest, then update #13 and #57 to it.
Devcontainer start-up and the 10-minute cold gate stay with platform-sw #57 and do not block this image PR. The branch is also 8 commits behind main.
Merge current main and address H-CI-1 review: enforce job-scoped permissions, main-only dispatch, add Jazzy twist_mux, and document the existing camera_ros runtime dependency. Preserve CycloneDDS. Co-authored-by: ChatGPT <noreply@openai.com> Signed-off-by: Arumuga Vignesh S R <129583467+wikki26@users.noreply.github.com>
|
H-CI-1 update — technical review requested on head
Per Alex's 22 Oct guidance, the next canonical GHCR publish should happen after approval/merge from @BotshareAI @panthera-momagdii: please re-review the code and publishing permission changes on this head. Thanks. |
|
H-CI-1 downstream evidence update (29 Oct 2026): image PR #41 is still at
This does not establish canonical |
panthera-momagdii
left a comment
There was a problem hiding this comment.
Reviewed current head 19266a51a84210b01e4486ac2cdb637bbb5cb8b7. The workflow now separates read-only PR validation from main-only GHCR publication with scoped permissions; action and base-image references are immutable; twist_mux is included; legacy rplidar_ros is excluded; and retaining camera_ros is supported by the current platform package/launch dependency. The image build and repository-quality runs pass. The current DCO check is also green through recorded manual approval. Approved for the baseline image change. After merge, publish from main, record the new digest, then update the manifest/devcontainer pins as already documented. This does not claim hardware or full H-CI-1 acceptance.
Summary
Adds the H-CI-1 ROS 2 Jazzy baseline CI image and GitHub Actions workflow for validation and authorized GHCR publication. Current update (29 Oct 2026): rebased in substance by merging the latest
maininto the feature branch and addressing Alex's 22 Oct review, at head19266a51a84210b01e4486ac2cdb637bbb5cb8b7.Changes
ros:jazzy-ros-baseby digest, installed platform/ROS build dependencies, and setRMW_IMPLEMENTATION=rmw_cyclonedds_cpp.ros-jazzy-twist-muxto support the planned velocity arbitration stack.ros-jazzy-camera-rosfor a confirmed current dependency:openamr-platform-sw/ros2/src/openamrobot_perception/package.xmldeclares<exec_depend>camera_ros</exec_depend>, andlaunch/camera.launch.pyinvokes the IMX708camera_ros/camera_nodefor the existing robot. This does not assert that the 2.0 Gemini 336L/Orbbec driver is implemented or validated.ros-jazzy-rplidar-ros; S3sllidar_ros2is a source-managed manifest/install dependency (see manifest PR Update README with sponsorship tiers and links #18), not hardware-qualified by this image.Validate CI image(fork-safe PR, onlycontents: read, never publishes) andPublish CI image(onlypushtomainor explicitworkflow_dispatchfrom main; scopedpackages: write).Validation and evidence
Publish CI imagejob was correctly skipped for this pull request.quality/test,ros-verifier-smoke, andrepository-quality / repository-quality. The optionalrepository-quality / quality/testreusable job is skipped on this repository, not evidence of platform test execution.linux/amd64. This is build evidence, not a passing platform test suite.ghcr.io/openamrobot/ci-ros2-jazzy@sha256:1653d4bdadce9999a48f73315e2da7aa667d3777579a97b0f1c12146667892d2originally published from the feature branch.Integration Gate — remaining work
twist_muxadded;camera_rosexception supported by actualpackage.xmldependency.main, record run ID and new digest, then update the manifest image pin and platform-sw devcontainer to the new canonical digest.Related work / acceptance boundary
micro_ros_agentrosdep resolution on Noble; the previous platform build succeeding does not resolve installation coverage.Safety impact
Changes a CI image and its publish/validation permissions only; no runtime motion, E-stop or physical robot behavior is modified.
twist_muxbeing available in the image does not establish that command arbitration is correctly integrated.AI assistance and provenance
ChatGPT assisted with the workflow/Dockerfile changes and review evidence. GitHub commits carry DCO sign-off; contributor agreement and project review checks remain authoritative.