Migrate BaseCamp devcontainer into EVerest devcontainer#63
Conversation
a64de81 to
8e24761
Compare
49d7f7a to
6dd08c0
Compare
There was a problem hiding this comment.
The general layout looks really good to me! Unfortunately I wasn't able to test it yet, I plan to do so in next feedback loop
- I made a lot of comments regarding naming and inline comments. I think this is quite important especially in a bash script environenment that is hard to read and error-prone.
- A lot of things should be moved to the dev-env-base image instead of being duplicated here.
-
Discuss: What happens when the nodered service is running and someone uses a run nodered script? I would vote for removing one of those options sop it is clear how to use nodered as service for everest sil
-
Documentation: The README.md is way to big. It's contents + REQUIREMENTS.md should be moved to the main documentation. The README should only include hints about what is inside this repository and roughly how to work with the repos contents and a link to the main documentation. So the usage of devcontainer tooling belongs to main documentation. I would suggest to split the devcontainer docs into the following chunks in the new upcoming docs structure:
- Tutorial: How to setup first devcontainer/dev environment and start sil or so
- How to guide: A bit advanced features as starting/stopping services using other profiles, using environemt variables, all the available commands
- Explanation: How does the devcontainer works: devcontainer vs docker compose, ports, all the stuff in the background which a developer would need to work on the tooling
- Reference: The Requirements could be protocol
led here as reference, but could also be omitted in my opinion. We only talked about F1-F7
- The old documentation should be removed/adapted
- Documentation should fit to the general getting started guide. Either it should replace it, extend it or link against each other
- Are there already related PRs in the corresping repositories I have overseen?
If you have any questions on my comments we also can set up a call to discuss it
devcontainer/template/.devcontainer/general-devcontainer/Dockerfile
Outdated
Show resolved
Hide resolved
devcontainer/template/.devcontainer/general-devcontainer/Dockerfile
Outdated
Show resolved
Hide resolved
devcontainer/template/.devcontainer/general-devcontainer/Dockerfile
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
The general layout looks really good to me! Unfortunately I wasn't able to test it yet, I plan to do so in next feedback loop
- I made a lot of comments regarding naming and inline comments. I think this is quite important especially in a bash script environenment that is hard to read and error-prone.
- A lot of things should be moved to the dev-env-base image instead of being duplicated here.
- What happens when the nodered service is running and someone uses a run nodered script? I would vote for removing one of those options sop it is clear how to use nodered as service for everest sil
- Documentation: The README.md is way to big. It's contents + REQUIREMENTS.md should be moved to the main documentation. The README should only include hints about what is inside this repository and roughly how to work with the repos contents and a link to the main documentation. So the usage of devcontainer tooling belongs to main documentation. I would suggest to split the devcontainer docs into the following chunks in the new upcoming docs structure:
- Tutorial: How to setup first devcontainer/dev environment and start sil or so
- How to guide: A bit advanced features as starting/stopping services using other profiles, using environemt variables, all the available commands
- Explanation: How does the devcontainer works: devcontainer vs docker compose, ports, all the stuff in the background which a developer would need to work on the tooling
- Reference: The Requirements could be protocolled here as reference, but could also be omitted in my opinion. We only talked about F1-F7
- The old documentation should be removed/adapted
- Documentation should fit to the general getting started guide. Either it should replace it, extend it or link against each other
- Are there already related PRs in the corresping repositories I have overseen?
If you have any questions on my comments we also can set up a call to discuss it
|
Please add proper title and description to this PR |
40a3c45 to
2688319
Compare
…yard) tool to handle the containers Signed-off-by: florinmihut <florinmihut1@gmail.com>
b95621c to
409eb33
Compare
Signed-off-by: Andreas Heinrich <andreas.heinrich@rwth-aachen.de>
andistorm
left a comment
There was a problem hiding this comment.
🐧 While still some aspects need a bit of rework this PR can be merged and further changes can be done in the monorepo
Uh oh!
There was an error while loading. Please reload this page.