Skip to content

Conversation

@glennvl
Copy link
Contributor

@glennvl glennvl commented Oct 28, 2025

Add a devcontainer with the required tools and vscode extensions plus configuration such that the same checks that happen in github workflows to check/lint the dockerfiles, scripts, readme, ... also happen on the fly while working on those files if the vscode IDE is used.

image

@netbrain
Copy link
Owner

devcontainers is not something i usually use, nor do i see the problem it solves (atleast for me) using nix flakes for dev env, is imo superior. So as it stands right now, I think this should instead be something developers setup themselves if need be. In any case, all the linting stuff is already in CI and will trigger on PR's, so this should really be enough.

However, i can be persuaded otherwise if reasoning is good.

@glennvl
Copy link
Contributor Author

glennvl commented Nov 4, 2025

devcontainers is not something i usually use, nor do i see the problem it solves (atleast for me) using nix flakes for dev env, is imo superior. So as it stands right now, I think this should instead be something developers setup themselves if need be. In any case, all the linting stuff is already in CI and will trigger on PR's, so this should really be enough.

However, i can be persuaded otherwise if reasoning is good.

The feedback for the developer is quite late if he has to wait until the linting is triggered by creating a pull request. I find it rather frustrating to have to go back to the code I just wrote and change all sorts of things to make the linters happy. And then I have to wait again for the results of the ci run to see if the changes I made were actually what the linter wanted. If not I have to do a few cycles of make a change, wait for ci, change again, ... It's way more ergonomic to have the linters provide feedback on-the-fly while writing the code in the first place. That way the fixes are very small and can be done immediately instead of having a whole list of violations to fix after the fact.

It's not so easy for the developer himself to find and install all the right tools, ide plugins, and properly configure them. It would involve looking into the ci scripts to see what tools are used. Then searching for the right ide plugins. And correctly configuring the plugins, which in our case is a bit more af a hassle than usual since the linter configuration files are not in the spot where the tools expect them to be. Again way more ergonomic for the developer to have this list of tools and configurations as part of the project.

As for using a nix flake vs a devcontainer, I'm not going to try to argue which is the better solution in general, I think it comes down mostly to personal perference and some edge cases. But consider the user base of this project. They all already have docker or podman installed on their system since it's required to run zwift. I'd wager most don't have nix installed and are not as familiar with it as containers. So most likely more ergonomic to provide a devcontainer than to have a nix flake.

Obviously it's up to you @netbrain to decide what you want to do with this. 😃

@netbrain
Copy link
Owner

netbrain commented Nov 4, 2025

I do want to try to maintain a small codebase, and adding all of these devtools causes complexity, no matter how small they are. They also create a precedence, so if x was merged then why not y. etc.

How about this, if we can get some more contributors backing this PR with a 👍 then, we'll merge it?

@glennvl
Copy link
Contributor Author

glennvl commented Nov 4, 2025

I do want to try to maintain a small codebase, and adding all of these devtools causes complexity, no matter how small they are. They also create a precedence, so if x was merged then why not y. etc.

How about this, if we can get some more contributors backing this PR with a 👍 then, we'll merge it?

Sure, sounds good to me.

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.

2 participants