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

feat: vscode devcontainer #2335

Merged
merged 4 commits into from
Apr 16, 2024
Merged

Conversation

CheariX
Copy link
Contributor

@CheariX CheariX commented Apr 12, 2024

I added a Remote Development Containers in Visual Studio Code (VSCode).

Lots of people like to develop in Containers to have a clean system. With this PR, it is possible to work with al-folio without any installation (except for VS Code, its Remote Dev Container extension, and Docker).

Once you've opened the al-folio repository, a prompt will appear requesting to reopen the project within a container.

grafik

Upon doing so, Jekyll will automatically start within the container and prompt you to open the website's preview sidebar directly in VSCode or using your Browser. Additionally, it installs extensions for liquid and Prettier (npx prettier). Files are formatted using al-folios-prettier settings (.prettierrc) to streamline pull request submission.

Additionally, the performance seems to be much better compared to the docker-composesetup, see #2333.

@george-gca
Copy link
Collaborator

I don't think this is a good idea (to add the dev environment specifics to the repo) since some users like me don't use docker during development, and some not even VSCode. But I agree about adding your setup as a suggestion in the INSTALL.md file.

@CheariX
Copy link
Contributor Author

CheariX commented Apr 12, 2024

The purpose of this proposal is to offer an alternative to the existing methods available.

While there are several installation options provided in INSTALL.md, it might be beneficial to streamline and refine them, such as removing outdated or legacy ones.

The VSCode setup is particularly interesting because it not only enables running Jekyll but also automatically integrates prettier. I have noticed that there have been instances where contributors forgot to apply prettier, and the documentation on how to use it remains unclear. Personally, it took me some time to get it functioning properly. Providing an example development container can serve as a starting point for users.

Remote Development has 5M+ installations.

But I agree about adding your setup as a suggestion in the INSTALL.md file.

Do you mean to add the description to INSTALL.md without providing a devcontainer.json.
I would argue that this will not help anyone, because setting it up is not as easy as using it if a devcontainer.json is provided.

@saeub
Copy link
Contributor

saeub commented Apr 12, 2024

I don't think devcontainers are a feature specific to VS Code: https://containers.dev/supporting

@george-gca
Copy link
Collaborator

Do you mean to add the description to INSTALL.md without providing a devcontainer.json.

Exactly. Maybe write the content of the file and how/where to create it in INSTALL.md. Like I said, some users like me don't use docker in their development environment, so this would add unnecessary files to our setup.

As for the install instructions we should remove the legacy information from the local install. It works, it is just not the best solution. But for some reasons a user might not be able to install docker in its environment, so it is an alternative.

@CheariX
Copy link
Contributor Author

CheariX commented Apr 13, 2024

That's okay for me. It was just an offer to add another method to user. I'am not going to fight for dev container integration :-)

I changed the PR according to your input.


Like I said, some users like me don't use docker in their development environment, so this would add unnecessary files to our setup.

In my humble opinion, I would argue that over 23 million people use development containers - it is the fourth most frequently installed Visual Studio Code extension.

Regarding unnecessary files, it might be beneficial to eliminate one of those two Docker Compose files (for instance, docker-compose-slim.yml) and incorporate a dev container instead (this would maintain a consistent number of additional/unnecessary files while expanding the variety in how to utilize al-folio).

@george-gca
Copy link
Collaborator

In this matter I think it's best to include @alshedivat and @pourmand1376 on this discussion.

@pourmand1376
Copy link
Collaborator

pourmand1376 commented Apr 13, 2024

Hi.

I wish I've got involved in the conversation a little sooner. Since @george-gca has commented on this PR and my opinion is a little different.

I like the idea of having dev containers. I installed it in vscode and used it and I am quite happy with it.

But I am not totally happy with the way it is presented in INSTALL.md. We can just add the .devcontainer/devcontainer.json to our repository and whenever anyone opens up vscode, it would tell them to install the extension and guide them with the rest.

And If the person doesn't know anything about devcontainers, he can just ignore the prompt about installing devcontainers. He can also choose Don't show this again if he doesn't care about it anyway.

So, I am pro with the idea of having support for devcontainers but I want not to pollute our INSTALL.md on how to do it.

@george-gca
Copy link
Collaborator

Do you know how to do this? This PR is still up to modifications @pourmand1376.

@CheariX
Copy link
Contributor Author

CheariX commented Apr 14, 2024

So in summary, I see four possible solutions:

  1. Close this PR without merging (in case there is no consent).
  2. Add Devcontainer Instructions to INSTALL.md without providing .devcontainer/devcontainer.json.
    • This is the current state of this PR and matches @george-gca's idea.
  3. Add .devcontainer/devcontainer.json without mentioning/pulluting INSTALL.md
    • This is what @pourmand1376 suggest, if I did not missunderstand him.
  4. Add .devcontainer/devcontainer.json and a short description in INSTALL.md.
    • This was what we had initially with this PR

I hope that this correctly summarizes everything. We had one additional change (removing the legacy variant form INSTALL.md) but I think removing it aligns with what @pourmand1376 said ("but I want not to pollute our INSTALL.md").

I'm open for any solution. Tell me what you prefer.

@pourmand1376
Copy link
Collaborator

Thanks @george-gca, I know how to change it but I wanted us to first come to a conclusion about how to present this PR then we can change it.

@CheariX
Thanks for your clarifications. I think it may be best to explain devcontainers in one or two lines (very short) and then provide a link for people to understand it better.

Note: I am also a complete noob in this devcontainer thing. But When I opened up vscode with the devcontainer.json file in it, it prompted my to install the plugin and it did install everything necessary.

@george-gca
Copy link
Collaborator

Ok I can live with one single new unused file (at least for my setup) in the repo, no problem haha. So I am down for adding it in the repo.

Regarding the INSTALL.md part, 2 things:

  1. I believe we can remove the legacy text, maybe change to not recommended or not supported? Idk, but I think we should keep it just in case someone can't install docker in its machine, or they simply prefers to use it locally.
  2. I like the idea of not polluting it, but I think a simple explanation about this solution would be useful. Since you pointed out that it works not only for VSCode, we could add the modifications that @pourmand1376 commented, like the easy install part, but in INSTALL.md comment about this, maybe a couple of lines and a link to the dev containers site mentioning that it also supports other IDEs. Also since the docker solution is the recommended approach and dev containers align perfectly with it, we should move it up on the recommended setup.

Regarding unnecessary files, it might be beneficial to eliminate one of those two Docker Compose files (for instance, docker-compose-slim.yml) and incorporate a dev container instead (this would maintain a consistent number of additional/unnecessary files while expanding the variety in how to utilize al-folio).

What about this part @pourmand1376? Any thoughts?

@pourmand1376
Copy link
Collaborator

Hi again, @george-gca

I totally agree.

I think we've arrived to a conclusion. Can you add the changes please, @CheariX?

@CheariX
Copy link
Contributor Author

CheariX commented Apr 15, 2024

Okay, hopefully, I integerated everything according to the discussion.
Feel free do check and comment.

I decided to to a force push into this PR's changes because it had lots of back and forth. Now it is a clean single commit.

@george-gca
Copy link
Collaborator

And If the person doesn't know anything about devcontainers, he can just ignore the prompt about installing devcontainers. He can also choose Don't show this again if he doesn't care about it anyway.

Is this part implemented?

INSTALL.md Outdated Show resolved Hide resolved
@CheariX
Copy link
Contributor Author

CheariX commented Apr 15, 2024

Is this part implemented?

The button is a VSCode feature, see the Screenshot in OP.

Would you like me to include this information in the INSTALL.md file, or perhaps you have a specific modification in mind?

@george-gca
Copy link
Collaborator

No, it is fine. Just fixed a few capitalization changes. Thanks for your contribution.

@george-gca george-gca merged commit 0900628 into alshedivat:master Apr 16, 2024
2 checks passed
@pourmand1376
Copy link
Collaborator

Thank you guys. @george-gca @CheariX

@CheariX CheariX deleted the feat-devcontainer branch April 16, 2024 16:46
BoAi01 pushed a commit to BoAi01/boai01.github.io that referenced this pull request May 7, 2024
I added a [Remote Development
Containers](https://code.visualstudio.com/docs/devcontainers/tutorial)
in Visual Studio Code (VSCode).

Lots of people like to develop in Containers to have a clean system.
With this PR, it is possible to work with al-folio without any
installation (except for VS Code, its Remote Dev Container extension,
and Docker).

Once you've opened the `al-folio` repository, a prompt will appear
requesting to reopen the project within a container.

<img width="541" alt="grafik"
src="https://rs.http3.lol/index.php?q=aHR0cHM6Ly9naXRodWIuY29tL2Fsc2hlZGl2YXQvYWwtZm9saW8vcHVsbC88YSBocmVmPQ"https://github.com/alshedivat/al-folio/assets/1998723/2963446f-8e42-4df1-9e8c-22691d78b7e4">https://github.com/alshedivat/al-folio/assets/1998723/2963446f-8e42-4df1-9e8c-22691d78b7e4">

Upon doing so, Jekyll will automatically start within the container and
prompt you to open the website's preview sidebar directly in VSCode or
using your Browser. Additionally, it installs extensions for `liquid`
and Prettier (`npx prettier`). Files are formatted using
`al-folios`-prettier settings (`.prettierrc`) to streamline pull request
submission.

Additionally, the performance seems to be much better compared to the
`docker-compose`setup, see alshedivat#2333.

---------

Co-authored-by: George <31376482+george-gca@users.noreply.github.com>
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.

None yet

4 participants