Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add minimal devcontainer setup #14038

Merged
merged 4 commits into from
Nov 13, 2024
Merged

feat: add minimal devcontainer setup #14038

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6a03fe
add minimal devcontainer setup
mcarbonne Nov 9, 2024
9ad1786
fix Makefile & update doc
mcarbonne Nov 9, 2024
335dc79
fix Makefile
mcarbonne Nov 9, 2024
d057c19
add warning regarding devcontainer + add newline at EOF
mcarbonne Nov 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .devcontainer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ARG BASEIMAGE=mcr.microsoft.com/devcontainers/typescript-node:22
FROM ${BASEIMAGE}
19 changes: 19 additions & 0 deletions .devcontainer/devcontainer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"name": "Immich devcontainers",
"build": {
"dockerfile": "Dockerfile",
"args": {
"BASEIMAGE": "mcr.microsoft.com/devcontainers/typescript-node:22"
}
Comment on lines +5 to +7
Copy link
Member

Choose a reason for hiding this comment

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

Why is this defined as an arg, and in two separate places?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I have no intention of adding anything else to this PR.
It is just an "initial" version of a working devcontainer that makes it easy to contribute to some components of immich (web, doc...) without having to setup a development environment. I'll let other people contribute !

Regarding BASEIMAGE, it was inspired by other open source projects => most likely it is easier to update the base image from the json file (rather than editing Dockerfile).
Also, for now, the Dockerfile is empty (just the base image), but it will allow to easily add software & tools later.

},
"customizations": {
"vscode": {
"extensions": [
"svelte.svelte-vscode"
]
Comment on lines +11 to +13
Copy link
Member

Choose a reason for hiding this comment

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

I think there's some more extensions worth adding, see https://immich.app/docs/developer/setup#vscode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's beyond my field of expertise. As I stated above, the purpose of my PR was to share my setup to ease contributions regarding web server (and doc).
I pretty sure that vscode/settings.json is still in use inside the dev container. Regarding missing extensions (maybe you are referring to flutter, ...), I've never used them so I won't be able to test anything.
If the change is easy, feel free to add a suggestion, otherwise, maybe open another PR ?

}
},
"forwardPorts": [],
"postCreateCommand": "make install-all",
"remoteUser": "node"
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add a new line to the end of the file :)

18 changes: 8 additions & 10 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ attach-server:
renovate:
LOG_LEVEL=debug npx renovate --platform=local --repository-cache=reset

MODULES = e2e server web cli sdk
MODULES = e2e server web cli sdk docs

audit-%:
npm --prefix $(subst sdk,open-api/typescript-sdk,$*) audit fix
Expand All @@ -48,11 +48,9 @@ install-%:
build-cli: build-sdk
build-web: build-sdk
build-%: install-%
npm --prefix $(subst sdk,open-api/typescript-sdk,$*) run | grep 'build' >/dev/null \
&& npm --prefix $(subst sdk,open-api/typescript-sdk,$*) run build || true
npm --prefix $(subst sdk,open-api/typescript-sdk,$*) run build
format-%:
npm --prefix $(subst sdk,open-api/typescript-sdk,$*) run | grep 'format:fix' >/dev/null \
&& npm --prefix $(subst sdk,open-api/typescript-sdk,$*) run format:fix || true
npm --prefix $* run format:fix
lint-%:
npm --prefix $* run lint:fix
check-%:
Expand All @@ -79,14 +77,14 @@ test-medium:
test-medium-dev:
docker exec -it immich_server /bin/sh -c "npm run test:medium"

build-all: $(foreach M,$(MODULES),build-$M) ;
build-all: $(foreach M,$(filter-out e2e,$(MODULES)),build-$M) ;
install-all: $(foreach M,$(MODULES),install-$M) ;
check-all: $(foreach M,$(MODULES),check-$M) ;
lint-all: $(foreach M,$(MODULES),lint-$M) ;
format-all: $(foreach M,$(MODULES),format-$M) ;
check-all: $(foreach M,$(filter-out sdk cli docs,$(MODULES)),check-$M) ;
lint-all: $(foreach M,$(filter-out sdk docs,$(MODULES)),lint-$M) ;
format-all: $(foreach M,$(filter-out sdk,$(MODULES)),format-$M) ;
audit-all: $(foreach M,$(MODULES),audit-$M) ;
hygiene-all: lint-all format-all check-all sql audit-all;
test-all: $(foreach M,$(MODULES),test-$M) ;
test-all: $(foreach M,$(filter-out sdk docs,$(MODULES)),test-$M) ;

clean:
find . -name "node_modules" -type d -prune -exec rm -rf '{}' +
Expand Down
1 change: 1 addition & 0 deletions docs/docs/developer/pr-checklist.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# PR Checklist

A minimal devcontainer is supplied with this repository. All commands can be executed directly inside this container to avoid tedious installation of the environment.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you mention that you can't run the Compose stack in this dev container, it'd be good to clarify that in the docs here as well.

Copy link
Contributor Author

@mcarbonne mcarbonne Nov 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed:
image

When contributing code through a pull request, please check the following:

## Web Checks
Expand Down
Loading