node: Add lv_font_conv to node package #1764
Conversation
|
Build size and comparison to main:
|
|
If we just need to do |
FintasticMan
force-pushed
the
npm_package
branch
6 times, most recently
from
b137ca2
to
db2120c
Compare
I've just done this, by installing the packages in the InfiniTime directory, and telling InfiniSim to also search there for the binaries. I've also updated the docker image to install the npm packages in the InfiniTime directory. |
| @@ -33,6 +33,8 @@ main() { | |||
| [ ! -d "$TOOLS_DIR/$NRF_SDK_VER" ] && GetNrfSdk | |||
| [ ! -d "$TOOLS_DIR/mcuboot" ] && GetMcuBoot | |||
|
|
|||
| npm i | |||
There was a problem hiding this comment.
why move the install from the docker file where it ran once to the build script where it has to run each build.
please try to move it back to the dockerfile
There was a problem hiding this comment.
To do that, it would need to clone the InfiniTime repo during the Docker image build. This doesn't really make sense, and I think that it doesn't make sense to need to rebuild the Docker image just to use updated npm packages. It also shouldn't affect the build time much, because if the packages are up to date it doesn't do anything.
There was a problem hiding this comment.
But it would require me to have a working internet connection for each build. Previously that was a requirement for just the docker image creation
I think you could use a
COPY packages.jsoncommand and delete it after the npm command again
Good point. In this case, npm actually runs even if there's no internet connection as long as the packages are up-to-date with the package-lock.json.
You're right, I could just download the package.json and package-lock.json from the repo and use those in the Docker image itself. I personally think that installing the modules in the repo and running npm i for every build is a better solution.
There was a problem hiding this comment.
I would also prefer to have all dependencies installed during the build of the Docker image rather than during the build of the project. This ensures that once the Docker image is generated and tested, it will be able to build the project no matter what.
There was a problem hiding this comment.
After looking into it, npm doesn't support installing packages globally from a package.json. If we do want the Docker container to contain the npm packages, we'll have to install them like we do now, without a package.json.
FintasticMan
force-pushed
the
npm_package
branch
from
db2120c
to
de1ac17
Compare
|
But it would require me to have a working internet connection for each build. Previously that was a requirement for just the docker image creation
I think you could use a `COPY packages.json` command and delete it after the npm command again
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
FintasticMan
force-pushed
the
npm_package
branch
from
de1ac17
to
cbd84ab
Compare
This means that these can be installed by simply running `npm i`. Also add node_modules to gitignore.
FintasticMan
force-pushed
the
npm_package
branch
from
cbd84ab
to
1f9e873
Compare
|
Might want to remove image conv from the title of the PR to clarify that it's no longer used, I was a bit confused at first |
FintasticMan
changed the title
|
Oh yeah, forgot to do that. |
This means that these can be installed by simply running
npm i.