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.

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

Open
wants to merge 53 commits into
base: develop
Choose a base branch
from
Open

Conversation

AVtheking
Copy link

@AVtheking AVtheking commented Aug 2, 2024

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

    • Introduced a new development container configuration for the Talawa API, enhancing the setup process for developers.
    • Added a multi-stage Dockerfile for optimized application builds and improved development tools.
    • Created a new docker-compose.yaml file to define multiple services, improving application orchestration.
    • Added a new section in the installation guide for setting up the Talawa-API server using Devpod.
  • Bug Fixes

    • Simplified pre-commit script to focus on TypeScript type checks.

Copy link

@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.

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

Commits

Files that changed from the base of the PR and between 413dc84 and 36b1820.

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.

.devcontainer/Dockerfile Outdated Show resolved Hide resolved
Copy link

@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.

Actionable comments posted: 1

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 36b1820 and bf8b54b.

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 run npm 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: ...un npm run setup to setup env file or you can start your server by running ``...

(COMMA_COMPOUND_SENTENCE)

INSTALLATION.md Outdated Show resolved Hide resolved
INSTALLATION.md Outdated Show resolved Hide resolved
INSTALLATION.md Outdated Show resolved Hide resolved
@@ -0,0 +1,36 @@
{
"name": "talawa api dev environment",
"dockerComposeFile": "docker-compose.yaml",
Copy link
Contributor

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?

Copy link
Author

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.

Copy link
Contributor

@xoldd xoldd left a 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.

@AVtheking
Copy link
Author

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?

Copy link

@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.

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

Commits

Files that changed from the base of the PR and between bf8b54b and 2f0bf4c.

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.

@palisadoes
Copy link
Contributor

palisadoes commented Aug 30, 2024

Is this ready to be reviewed?

@AVtheking
Copy link
Author

Is this ready to be reviewed?

yes

Copy link
Contributor

@palisadoes palisadoes left a 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)
Copy link
Contributor

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.

Copy link
Author

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 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

We have a section in the table of contents for installing prerequisites. If we need to install different prerequisites depending on whether we use devcontainer or not, then we need the prerequisites section to be split in two for each scenario.

image

INSTALLATION.md Outdated Show resolved Hide resolved
INSTALLATION.md Outdated Show resolved Hide resolved
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.
Copy link
Contributor

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

Copy link
Author

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.

Copy link
Contributor

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.

Copy link
Author

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.

@palisadoes
Copy link
Contributor

  1. Even with DevContainer you have to do setup, so it is not the desired one click ease of use.
  2. I noticed that DevContainer has an AppImage. Could we use something similar to get to a one click solution?

Copy link

@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.

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

Commits

Files that changed from the base of the PR and between 2f0bf4c and b868a11.

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"
done

Length 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)

INSTALLATION.md Show resolved Hide resolved
@varshith257
Copy link
Member

varshith257 commented Sep 1, 2024

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.

@AVtheking
Copy link
Author

  1. Even with DevContainer you have to do setup, so it is not the desired one click ease of use.

    1. I noticed that DevContainer has an AppImage. Could we use something similar to get to a one click solution?

Do you mean to create a AppImage like thing for the api server ?

@AVtheking
Copy link
Author

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.

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.

@AVtheking
Copy link
Author

  1. Even with DevContainer you have to do setup, so it is not the desired one click ease of use.

    1. I noticed that DevContainer has an AppImage. Could we use something similar to get to a one click solution?

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.

Copy link

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.

@github-actions github-actions bot added the no-pr-activity No pull request activity label Sep 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-pr-activity No pull request activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants