MAINT : Moved Docker images repo to docker folder in main branch#3177
MAINT : Moved Docker images repo to docker folder in main branch#3177Helion55 wants to merge 16 commits intonebari-dev:mainfrom
Conversation
Added Prettier and pre-commit hooks for autoformatting and linting.
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
Thanks for separating that work into separate PRs @Helion55. Can you get the "pre-commit" check to pass before we review. Thank you! |
|
Hi @Adam-D-Lewis, The pre-commit checks are passed. Is it fine to proceed now? |
dcmcand
left a comment
There was a problem hiding this comment.
Hi @Helion55,
Thanks so much for this PR. This will be really useful when it is merged. I left specific comments inline, but also want to call out that currently if someone has a branch that includes changes to the images, those updated images will not be used in integration tests. This eliminates the main advantage of this move. Please update the integration tests to use the new images if changes are made to the docker images.
There was a problem hiding this comment.
Since this change is unrelated to the current issue, please move it out of this pr.
| branches: | ||
| - "*" | ||
| paths: | ||
| - "Dockerfile" |
There was a problem hiding this comment.
These paths won't work for this repo
|
|
||
| on: | ||
| pull_request: | ||
| paths: |
There was a problem hiding this comment.
These paths won't work with this repo
| - name: "Build docker images 🐳" | ||
| uses: docker/build-push-action@v3 | ||
| with: | ||
| context: . |
There was a problem hiding this comment.
This context is not correct for this repo
.github/workflows/test_images.yaml
Outdated
| - name: Build Image 🛠 | ||
| uses: docker/build-push-action@v3 | ||
| with: | ||
| context: . |
There was a problem hiding this comment.
This context is incorrect for this repo
| dockerfile: | ||
| - jupyterlab | ||
| - jupyterhub | ||
| - dask-worker |
There was a problem hiding this comment.
Is there a reason that you aren't including nebari-workflow-controller here?
There was a problem hiding this comment.
Line 67 of this file in now out of date and should be updated.
There was a problem hiding this comment.
References to the old docker repo need to be updated here
.github/workflows/test_images.yaml
Outdated
| on: | ||
| pull_request: | ||
| paths: | ||
| - "Dockerfile.*" |
There was a problem hiding this comment.
This path is incorrect, there is a single dockerfile, so Dockerfile.* will never match
|
Hello @dcmcand, any update on this? |
Reference Issues or PRs
Fixes #2871
What does this implement/fix?
Moved docker-images repo back to main nebari's repo. Changes made,
dockerand moved the necessary files from docker repository..github/workflowsfolder by adding thebuild-push-docker.yamlandtest-images.yamlfiles from docker repo's workflow folder.documentation.ymlandRFD.mdfiles to.github/ISSUE_TEMPLATEfolder..pre-commit-config.yamlfile.Testing
no
no
Any other comments?
no