-
Notifications
You must be signed in to change notification settings - Fork 52
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 support #709
base: master
Are you sure you want to change the base?
Devcontainer support #709
Conversation
This looks fantastic! I'll give it a test tomorrow morning (local) to see if I can find any quirks. What do you think about adding what you typed up in the PR to a |
@codyro Good idea, have added! |
- You can find explanations of the changes in AlmaLinux#709
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've made some comments/suggested changes. Feel free to use/tweak as you see fit.
I've also made a PR to your repo (alaurie#1) with the changes I commented on.
.devcontainer/devcontainer.json
Outdated
"dockerfile": "Dockerfile", | ||
"args": { | ||
"VARIANT": "hugo", | ||
"VERSION": "latest" |
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.
While this should be fine we should verify the site runs fine on the latest Hugo version (v0.139.4
) as we currently pin the production site to 0.98.0
. This was done at the time to ensure no surprise breakages between versions.
Similar with Python--we pin version 3.11
, however we should be able to use the 3.12
in repositories (or we could pin 3.9
to match the devcontainer).
Ultimately I'd like to have a 1:1 environment with our production, so we have two options:
- Update the devcontainer versions to match production (very old at this point)
- Verify everything runs cleanly on the latest versions of Hugo and Python from the repos (3.12.x) and update the production site to match
I'm thinking option 2 is the move as I'm pretty sure most people working on the site aren't using an ancient version of Hugo..
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.
as a note: I'm not sure the pinning you mentioned is actually working as intended. I recently (maybe as many as a few months ago) hit some snags due to outdated hugo installed on my laptop. I had to upgrade so that what I was doing would match the errors I was seeing as it deployed to dev.
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 think you're correct @bennyvasquez. The latest run I can see in GitHub actions uses the latest version 0.140.0
https://github.com/AlmaLinux/almalinux.org/actions/runs/12401185107/job/34620009233
Several tweaks to AlmaLinux#709
Revert "Several tweaks to AlmaLinux#709"
Several tweaks to AlmaLinux#709
- Ensures workspace is properly mounted *before* `hugo server` is ran - It attempted to run the command before the `config.yaml` was available
- Accidental nohup log committed - Test image I built
Fix hugo failing to start in the background successfully
Assuming we think this is ready to merge, I'll want to get a few community members to test it to ensure there are no lingering quirks and engage with anyone interested in helping out contributing (I just intend to ask in MatterMost, nothing fancy). Once we get some confirmation that everything works flawlessly, let's merge this as-is and then submit a new issue to add documentation. Or we could add documentation to this PR if we feel exceptionally motivated. We could add a page (or multiple) on the wiki about how to use the devcontainers in projects to contribute (you said you're considering wiki after this, and I'll be looking to use it elsewhere). |
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.
Teamwork makes the dream work 🤝
Documentation is critical and should be added along with these updates, so I'd like it to be part of this PR. |
It doesn't affect production. It's essentially the equivalent of adding a utility script that's unused. The scope of it being in a single PR isn't really realistic. Splitting it up will be more effective in this case. |
rm hugo.tar.gz | ||
|
||
# Create a non-root user for development | ||
ARG USERNAME=vscode |
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'd propose to call the default user "alma" or "hugosite", something more meaningful in my opinion than "vscode"
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 default base image templates (and most other devcontainer documentation) leverage the vscode
user by default--so I think anyone familiar with devcontainers wouldn't be confused.
I don't see any reason we cannot switch it--we need to make sure with any future modifications to remember that we're using a "non-standard" username (or just always reference the UID
I guess?).
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.
Ok, I wasn't aware that it is a general "practice" and I'm fine with it
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 don't know if it's "best practice" as much as "path of least resistance" 😄.
It would look better with a different username. I just wanted to provide context as to why that was likely chosen :).
It's up to @alaurie if he wants to rotato potato it or not.
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 switch it the consensus is to do so for Alma. Like @codyro said, its just a general default on a lot of prefab containers on Docker and various other registries.
Is this intended only for docker users? |
This pull request introduces initial support for developing the Hugo site on AlmaLinux within a devcontainer. This setup enhances the development experience by providing a consistent, pre-configured environment directly from Visual Studio Code:
Devcontainer Features:
VSCode Extensions: Automatically installs the following extensions for optimal Hugo development:
budparr.language-hugo-vscode - Hugo syntax support
ms-python.python - Python development support
esbenp.prettier-vscode - Code formatting
yzhang.markdown-all-in-one - Enhanced Markdown editing
tamasfe.even-better-toml - TOML language support
Terminal Configuration: Sets the integrated terminal to /bin/bash for a familiar shell experience.
Port Forwarding: Configures port 1313 to be forwarded for Hugo's live server.
User Configuration: Sets remoteUser to vscode, providing a secure, non-root user environment.
Dockerfile Setup:
This setup aims to streamline the development process by automating the environment setup, ensuring consistency across different machines, and reducing the time spent on configuration. Please review and provide feedback or approval to merge.