InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.74k stars 936 forks source link

Improve devcontainer #1497

Open emosenkis opened 1 year ago

emosenkis commented 1 year ago
Riksu9000 commented 1 year ago

I don't think the currently active maintainers use the devcontainer, so you'll have to be more descriptive about the purpose of these changes, so we can properly review them.

BloodStainedCrow commented 1 year ago

Currently when developing InfiniTime inside the devcontainer, neither lv_font_conv nor npm(to install lv_font_conv manually) is installed. This results in being unable to compile. With this change both npm and lv_font_conv are installed automatically, and compilation succeeds.

Question: Might it be more reasonable to only install npm and handle installing lv_font_conv via a vscode task. Just like installing the git submodules is handled?

@emosenkis What is cbors for?

FintasticMan commented 1 year ago

From reading the docs, I think it should be possible to adapt the devcontainer to use the main Dockerfile, which will reduce code duplication and maintenance cost. I don't use VS Code myself though, so I don't think I can help too much with that. If someone who is more familiar with devcontainers can shed some light, that would be appreciated.

BloodStainedCrow commented 1 year ago

You are totally correct. We currently seem to have two almost identical copies of Dockerfile and build.sh script in /.devcontainer and /docker. The differences seem to be:

None of these should be a problem to unify these two Dockerfiles. I would propose to add a argument so that the parts only necessary for devcontainer are only installed when needed. On the same note I would propose unifying the two build.sh scripts in /.devcontainer and /docker for that someone else will have to have a look at them to determine which is correct since they have weird differences like gcc-version where I do not know which is correct.

BloodStainedCrow commented 1 year ago

If any currently active maintainers use docker for compilation I would assume everything used there is more accurate/up to date.

FintasticMan commented 1 year ago

The main Docker image is up-to-date, and is used for the GitHub CI.

BloodStainedCrow commented 1 year ago

Perfect! So we only need to conditionally add the required packages for the devcontainer and create another user.