Skip to content

Revert "Docker"#58

Merged
jlee733 merged 1 commit intomainfrom
revert-38-jlee-docker
Mar 20, 2026
Merged

Revert "Docker"#58
jlee733 merged 1 commit intomainfrom
revert-38-jlee-docker

Conversation

@jlee733
Copy link
Copy Markdown

@jlee733 jlee733 commented Mar 18, 2026

-This PR is being reverted to keep this repo in sync with the Ando Lab's mdx2 repo
-The files being deleted will be moved to the 'mdx2-workflows' repo
-Reverts #38

Summary by CodeRabbit

  • Chores
    • Updated Docker build configuration to support multiple architectures.
    • Modified development environment containerization approach.

@jlee733 jlee733 requested review from bielj, saada and spmeisburger March 18, 2026 20:21
@jlee733 jlee733 self-assigned this Mar 18, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 18, 2026

Walkthrough

The PR updates the GitHub Actions Docker build workflow to reference a new Dockerfile location at .github/Dockerfile, adds QEMU setup for multi-platform builds, removes cache configuration, and deletes the old root Dockerfile that previously orchestrated a multi-stage Python/Jupyter environment setup.

Changes

Cohort / File(s) Summary
CI Workflow Configuration
.github/workflows/docker.yml
Added QEMU setup step, updated Dockerfile reference to .github/Dockerfile, and removed cache-from/cache-to configuration.
Root Dockerfile
dockerfile
Removed multi-stage Dockerfile that configured Python 3.10 environment with micromamba, conda packages (nexpy, jupyterlab, dials, xia2), and JupyterHub stack for port 8888.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • spmeisburger

Poem

🐇 A Dockerfile moves to a hidden place,
.github/ becomes its new space,
QEMU steps in to help build wide,
With cache swept clean, we let it glide,
Multi-platform dreams come true! 🐳

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Revert "Docker"' clearly and specifically summarizes the main change: reverting a previous Docker-related PR. It is concise, directly related to the changeset (which removes Docker configuration files), and provides meaningful context.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch revert-38-jlee-docker
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Tip

You can disable the changed files summary in the walkthrough.

Disable the reviews.changed_files_summary setting to disable the changed files summary in the walkthrough.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
.github/workflows/docker.yml (2)

37-43: Restore Buildx cache to avoid expensive layer rebuilds on each CI run.

The workflow is missing cache-from and cache-to settings. The .github/Dockerfile has multiple expensive operations (apt-get install, micromamba environment creation, pip installs of external packages and the dev package) that rebuild from scratch on every run without caching. Add GitHub Actions cache:

Suggested cache settings
       - name: Build and push
         uses: docker/build-push-action@v6
         with:
           context: .
           file: .github/Dockerfile
+          cache-from: type=gha
+          cache-to: type=gha,mode=max
           push: ${{ github.ref == 'refs/heads/main' }}
           tags: ${{ steps.tags.outputs.tags }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/docker.yml around lines 37 - 43, The Build and push step
using docker/build-push-action@v6 lacks Buildx layer caching; update the step
(the "Build and push" action) to add the cache-from and cache-to inputs and
enable loading/pushing of the cache so expensive layers in .github/Dockerfile
(apt installs, micromamba, pip installs) are reused across runs; specifically,
add cache-from: type=gha,scope=${{ github.ref }} and cache-to:
type=gha,mode=max,scope=${{ github.ref }} (or equivalent buildx cache settings)
and ensure push: ${{ github.ref == 'refs/heads/main' }} remains so cache is
uploaded on main.

22-24: QEMU setup is unused without declaring build platforms.

Line 22-23 adds QEMU emulation, but the build step at line 37-43 does not specify platforms. QEMU is only beneficial when building for multiple architectures. Either remove the QEMU setup or add explicit multi-arch platforms to the build configuration.

Suggested adjustment (if multi-arch is intended)
       - name: Build and push
         uses: docker/build-push-action@v6
         with:
           context: .
           file: .github/Dockerfile
+          platforms: linux/amd64,linux/arm64
           push: ${{ github.ref == 'refs/heads/main' }}
           tags: ${{ steps.tags.outputs.tags }}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/docker.yml around lines 22 - 24, The QEMU setup step using
docker/setup-qemu-action@v3 is unused because the Docker build step (the
docker/build-push-action step) does not declare `platforms`; either remove the
"Set up QEMU" step entirely or update the Docker build step to enable multi-arch
builds by adding a `platforms:` input (e.g. platforms: linux/amd64,linux/arm64)
to the docker/build-push-action invocation so QEMU emulation is actually
required and used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.github/workflows/docker.yml:
- Around line 37-43: The Build and push step using docker/build-push-action@v6
lacks Buildx layer caching; update the step (the "Build and push" action) to add
the cache-from and cache-to inputs and enable loading/pushing of the cache so
expensive layers in .github/Dockerfile (apt installs, micromamba, pip installs)
are reused across runs; specifically, add cache-from: type=gha,scope=${{
github.ref }} and cache-to: type=gha,mode=max,scope=${{ github.ref }} (or
equivalent buildx cache settings) and ensure push: ${{ github.ref ==
'refs/heads/main' }} remains so cache is uploaded on main.
- Around line 22-24: The QEMU setup step using docker/setup-qemu-action@v3 is
unused because the Docker build step (the docker/build-push-action step) does
not declare `platforms`; either remove the "Set up QEMU" step entirely or update
the Docker build step to enable multi-arch builds by adding a `platforms:` input
(e.g. platforms: linux/amd64,linux/arm64) to the docker/build-push-action
invocation so QEMU emulation is actually required and used.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d7b7547f-2303-4e6d-9a14-a15df13ad0dd

📥 Commits

Reviewing files that changed from the base of the PR and between a71f2f3 and ada3095.

📒 Files selected for processing (2)
  • .github/workflows/docker.yml
  • dockerfile
💤 Files with no reviewable changes (1)
  • dockerfile

@jlee733 jlee733 merged commit 9570c85 into main Mar 20, 2026
2 checks passed
@jlee733 jlee733 deleted the revert-38-jlee-docker branch March 20, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant