-
-
Notifications
You must be signed in to change notification settings - Fork 804
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.
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
Devcontainer #2429
base: develop
Are you sure you want to change the base?
Devcontainer #2429
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
.devcontainer/Dockerfile (1)
8-8
: Clarify the purpose of the CMD instruction.The
CMD ["tail", "-f", "/dev/null"]
is used to keep the container running. If this is for development purposes, consider adding a comment to clarify its intent.# Keep the container running for development purposes CMD ["tail", "-f", "/dev/null"]
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .devcontainer/Dockerfile (1 hunks)
Additional comments not posted (1)
.devcontainer/Dockerfile (1)
1-1
: Consider the base image choice.Using
debian:bookworm
as the base image is suitable for a stable and up-to-date environment. Ensure that this aligns with the project's requirements for compatibility and security.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- Dockerfile.dev (1 hunks)
- INSTALLATION.md (2 hunks)
Files skipped from review due to trivial changes (1)
- Dockerfile.dev
Additional context used
Markdownlint
INSTALLATION.md
18-18: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level(MD005, list-indent)
18-18: Expected: 0; Actual: 1
Unordered list indentation(MD007, ul-indent)
LanguageTool
INSTALLATION.md
[grammar] ~293-~293: The verb ‘provide’ is plural. Did you mean: “provides”? Did you use a verb instead of a noun?
Context: ... ``` ## Install Using Devpod This guide provide step by step guide to setup a Talawa-Ap...(PLURAL_VERB_AFTER_THIS)
[grammar] ~293-~293: Did you mean the adjective or adverb “step-by-step” (spelled with hyphens)?
Context: ...Install Using Devpod This guide provide step by step guide to setup a Talawa-Api server usin...(STEP_BY_STEP_HYPHEN)
[grammar] ~293-~293: The word “setup” is a noun. The verb is spelled with a white space.
Context: ...his guide provide step by step guide to setup a Talawa-Api server using devpod. Here...(NOUN_VERB_CONFUSION)
[uncategorized] ~300-~300: Possible missing comma found.
Context: ...t your desired ide . 5. After the above steps you will get the talawa api server setu...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~301-~301: Possible missing comma found.
Context: ...n your desired ide. 6. After the Ide is opened runnpm run setup
to setup env fi...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~301-~301: Use a comma before ‘or’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...unnpm run setup
to setup env file or you can start your server by running ``...(COMMA_COMPOUND_SENTENCE)
@@ -0,0 +1,36 @@ | |||
{ | |||
"name": "talawa api dev environment", | |||
"dockerComposeFile": "docker-compose.yaml", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't you utilize the already existing docker-compose.dev.yaml
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I was using the compose file outside the .decontainer.json file it was having some issues related to the project directory, that's why I used the file inside it. If I move the already existing file to that folder, developers can have issues, if they use docker for db.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The core idea behind devcontainers is to bring development environment as close to production as is feasible if the project is making using of containers for its workflows. Read this:- https://12factor.net/dev-prod-parity
Reproducibility is a core requirement for detecting bugs easily. As such the base dependencies like the linux distribution and its exact version, node.js and its exact version and package managers like npm/yarn/pnpm and their exact version etc. must be the same in development and production environment. I would suggest debian bookworm as the linux distribution because it has better compatibility with the wider software ecosystem compared to alpine linux, for node.js I would suggest using latest lts version(20.17.0 as of writing this comment), npm comes pre-bundled with node.js so that isn't required.
Same goes for the services used within the docker compose file.
Now on top of that you can add additional stuff required in development environment that aren't required in the production like git, vim, neovim, github cli etc., as these tools are required by developers to work on the project within the container . It is better to strictly version these dependencies for reproducibility as well but it isn't a hard requirement.
So I need to specify specific version in the script right? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
INSTALLATION.md (4)
18-18
: Fix list indentation.The list item indentation is inconsistent. Ensure that all list items at the same level have the same indentation.
- - [Installation Using Devpod](#install-using-devpod) + - [Installation Using Devpod](#install-using-devpod)Tools
Markdownlint
18-18: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level(MD005, list-indent)
18-18: Expected: 0; Actual: 1
Unordered list indentation(MD007, ul-indent)
292-293
: Refine Devpod installation instructions for clarity and grammar.The new section on Devpod installation could benefit from clearer step-by-step instructions and grammatical corrections to enhance readability and accuracy.
-This guide provides a step-by-step guide to setting up a Talawa-Api server using Devpod. +This guide provides a step-by-step guide to setting up a Talawa-API server using Devpod.
297-298
: Clarify provider instructions and fix macOS capitalization.Clarify the instructions for adding a provider and fix the capitalization of macOS.
-2. Add a provider to Devpod. For CLI, see [here](https://devpod.sh/docs/getting-started/quickstart-devpod-cli#add-a-provider) and for the GUI app, see [here](https://devpod.sh/docs/getting-started/quickstart-vscode#add-a-provider). Use Docker which you can install from their [official documentation](https://docs.docker.com/engine/install/) or docker compatible provider like podman, colima(better compatibility with macos). +2. Add a provider to Devpod. For CLI, see [here](https://devpod.sh/docs/getting-started/quickstart-devpod-cli#add-a-provider) and for the GUI app, see [here](https://devpod.sh/docs/getting-started/quickstart-vscode#add-a-provider). Use Docker, which you can install from their [official documentation](https://docs.docker.com/engine/install/), or a Docker-compatible provider like Podman or Colima (better compatibility with macOS).Tools
LanguageTool
[grammar] ~298-~298: The operating system from Apple is written “macOS”.
Context: ...odman, colima(better compatibility with macos). 3. Run the following command: ```devp...(MAC_OS)
302-302
: Fix grammar and clarify instructions.Improve the clarity and grammar of the instructions for setting up the environment file and starting the server.
-6. Once the IDE is open, run ```npm run setup``` to set up the environment file, or start your server by running ```npm run dev```. +6. Once the IDE is open, run ```npm run setup``` to set up the environment file, or start your server by running ```npm run dev```.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- .devcontainer/Dockerfile (1 hunks)
- INSTALLATION.md (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- .devcontainer/Dockerfile
Additional context used
Markdownlint
INSTALLATION.md
18-18: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level(MD005, list-indent)
18-18: Expected: 0; Actual: 1
Unordered list indentation(MD007, ul-indent)
LanguageTool
INSTALLATION.md
[grammar] ~298-~298: The operating system from Apple is written “macOS”.
Context: ...odman, colima(better compatibility with macos). 3. Run the following command: ```devp...(MAC_OS)
Additional comments not posted (1)
INSTALLATION.md (1)
299-299
: Update repository URI for production.The repository URI should target the
develop
branch for production use.-3. Run the following command: ```devpod up https://github.com/PalisadoesFoundation/talawa-api@develop```, or follow the instructions for CLI [here](https://devpod.sh/docs/developing-in-workspaces/create-a-workspace#git-repository) and for the GUI app [here](https://devpod.sh/docs/getting-started/quickstart-vscode#start-a-workspace-with-vs-code). +3. Run the following command: ```devpod up https://github.com/PalisadoesFoundation/talawa-api@develop```, or follow the instructions for CLI [here](https://devpod.sh/docs/developing-in-workspaces/create-a-workspace#git-repository) and for the GUI app [here](https://devpod.sh/docs/getting-started/quickstart-vscode#start-a-workspace-with-vs-code).Ensure the repository URI is correct for production use.
Is this ready to be reviewed? |
yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More work needs to be done on the instructions. I need to be able to follow the instructions without having to know insider steps.
It is promising work. I hope this will make installation much easier and faster.
INSTALLATION.md
Outdated
|
||
Follow these steps: | ||
|
||
1. Install the Devpod GUI application or Devpod CLI. [Learn more](https://devpod.sh/docs/getting-started/install) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this method independent of installing node.js and typescript as prerequisites? If it is, then it should be its own section. Think of the end user, not everyone is familiar with this technology.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes it is independent of installing node. js and ts , I didn't get with the point it should be its own section , could you clearify it more ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
INSTALLATION.md
Outdated
1. Install the Devpod GUI application or Devpod CLI. [Learn more](https://devpod.sh/docs/getting-started/install) | ||
2. Add a provider to Devpod. For CLI, see [here](https://devpod.sh/docs/getting-started/quickstart-devpod-cli#add-a-provider) and for the GUI app, see [here](https://devpod.sh/docs/getting-started/quickstart-vscode#add-a-provider). Use Docker which you can install from their [official documentation](https://docs.docker.com/engine/install/) or docker compatible provider like podman, colima(better compatibility with macos). | ||
3. Run the following command: ```devpod up https://github.com/PalisadoesFoundation/talawa-api@develop```, or follow the instructions for CLI [here](https://devpod.sh/docs/developing-in-workspaces/create-a-workspace#git-repository) and for the GUI app [here](https://devpod.sh/docs/getting-started/quickstart-vscode#start-a-workspace-with-vs-code). | ||
4. Select your desired IDE. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the user isn't a developer then they won't have an IDE. How will this help? Is there a solution to non-technical users to get the app functional?
More work needs to be done on the instructions. I need to be able to follow the instructions without having to know insider steps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can have the api running without using ide , but for that it require little bit of change, I did just not spin up the api server using this is for developer as they may want to customize it according to their need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to be clearer. You are assuming the only user will be a developer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest first let us test it for developers, then we can move to the users ease.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
INSTALLATION.md (2)
339-339
: Specify language for fenced code blocks.The fenced code block starting at line 339 does not specify a language, which can affect syntax highlighting and readability in markdown viewers and editors.
Specify the language to enhance readability:
- ``` + ```bash https://github.com/PalisadoesFoundation/talawa-api@develop<details> <summary>Tools</summary> <details> <summary>Markdownlint</summary><blockquote> 339-339: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </blockquote></details> </details> --- `314-314`: **Add a comma for clarity.** There's a missing comma after "ide" which can improve the readability of the sentence. Add a comma to enhance the flow of the text: ```diff - - To choose your ide refer to [Devpod CLI ide guide](https://devpod.sh/docs/developing-in-workspaces/connect-to-a-workspace#vs-code) + - To choose your ide, refer to [Devpod CLI ide guide](https://devpod.sh/docs/developing-in-workspaces/connect-to-a-workspace#vs-code)
Tools
LanguageTool
[uncategorized] ~314-~314: Possible missing comma found.
Context: ...Select Your IDE*: - To choose your ide refer to [Devpod CLI ide guide](https:/...(AI_HYDRA_LEO_MISSING_COMMA)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- .devcontainer/Dockerfile (1 hunks)
- INSTALLATION.md (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- .devcontainer/Dockerfile
Additional context used
Markdownlint
INSTALLATION.md
18-18: Expected: 0; Actual: 1
Inconsistent indentation for list items at the same level(MD005, list-indent)
18-18: Expected: 0; Actual: 1
Unordered list indentation(MD007, ul-indent)
339-339: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
LanguageTool
INSTALLATION.md
[uncategorized] ~314-~314: Possible missing comma found.
Context: ...Select Your IDE*: - To choose your ide refer to [Devpod CLI ide guide](https:/...(AI_HYDRA_LEO_MISSING_COMMA)
Additional comments not posted (1)
INSTALLATION.md (1)
292-357
: Review the new Devpod installation section.The new section provides a detailed guide for setting up the Talawa-API using Devpod. It includes both CLI and GUI instructions, which is beneficial for accommodating different user preferences. The instructions are clear and provide direct links to necessary resources, which should help users follow the steps without confusion.
However, ensure that all links are up-to-date and accessible to avoid dead links in the documentation.
Verification successful
All links are accessible and up-to-date.
The links in the Devpod installation section of the
INSTALLATION.md
file have been verified and are currently accessible, returning a 200 OK status. No dead links were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for dead links in the Devpod installation section. # Test: Fetch the HTTP status codes for each link. Expect: HTTP 200 OK for all. links=( "https://devpod.sh/docs/getting-started/install#optional-install-devpod-cli" "https://docs.docker.com/engine/install/" "https://devpod.sh/docs/getting-started/quickstart-devpod-cli#add-a-provider" "https://devpod.sh/docs/developing-in-workspaces/create-a-workspace#git-repository" "https://devpod.sh/docs/developing-in-workspaces/connect-to-a-workspace#vs-code" "https://devpod.sh/docs/getting-started/install" "https://devpod.sh/docs/getting-started/quickstart-vscode#add-a-provider" "https://devpod.sh/docs/getting-started/quickstart-vscode#start-a-workspace-with-vs-code" ) for link in "${links[@]}"; do status=$(curl -o /dev/null -s -w "%{http_code}\n" "$link") echo "Link: $link - Status: $status" doneLength of output: 3333
Tools
LanguageTool
[uncategorized] ~314-~314: Possible missing comma found.
Context: ...Select Your IDE*: - To choose your ide refer to [Devpod CLI ide guide](https:/...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
339-339: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
I see that both Devcontainer and Devpod are included in the PR. While both tools have their strengths, could you clarify the specific use cases for each? Devcontainer already provides a consistent local environment, so I am curious about the rationale for including Devpod as well. Simplifying the setup might help reduce complexity for new developers. If the focus is on achieving a true one-click setup experience for local development, Devpod may not be necessary. if remote capabilities are necessary, consider using Devpod as the standalone setup tool with DevContainer as an optional setup. |
Do you mean to create a AppImage like thing for the api server ? |
see Devpod uses devContainer.json for its underlying implementation , I agree we can achieve this with the help of devcontainer but it is specific to vs code only , with devpod we can create this environment in any ide or without any ide. |
by the way , devpod requires just initial setup that is installing of devpod setting up of provider.... This would still be required by the user to host the server , with devpod it is just one command and the server would spin up with all the dependencies it requires. |
This pull request did not get any activity in the past 10 days and will be closed in 180 days if no update occurs. Please verify it has no conflicts with the develop branch and rebase if needed. Mention it now if you need help or give permission to other people to finish your work. |
What kind of change does this PR introduce?
Created a devcontainer configuration to build the projects using devpods
Issue Number:
Fixes #2385
Did you add tests for your changes?
No
If relevant, did you update the documentation?
Summary
Created configuration for the devpod to open the project in the prebuilt development environment.
Does this PR introduce a breaking change?
No
Have you read the contributing guide?
Summary by CodeRabbit
New Features
docker-compose.yaml
file to define multiple services, improving application orchestration.Bug Fixes