Conversation
e22551f to
82f82ff
Compare
|
Thanks @strawburster. Results for me on Linux with Node.js v24 on Before:
After (first run to build cache, second run to test):
I rarely run lint in my day to day DSpace maintenance activities. The huge benefit here seems to be speeding up our CI runs. Are there any downsides? P.S. props for using podman instead of Docker. I do too! |
c39bcdb to
bb8516c
Compare
|
Hey, thanks for testing! Actually, I wasn't concerning myself with the CI pipeline, but just for local development. Do we use the main Dockerfile for CI? It says it's for development purposes. If we do, then, I don't think this change will actually improve speed because creating the lint cache takes just as much time, if not more, than linting already, so the time gained in the actual test portion is more than lost in the build portion. I usually run the dev server and linter inside the container, and bind-mount just the source code, that way I can avoid installing node_modules dependencies on my host system. The point of adding the lint-cache command to the dockerfile was so that I could easily delete and recreate containers, with the cache file stored in the image, and the first lint task I run will be very fast. One of the issues I ran into is that the metadata between the image files and the bind-mount files is usually not correct, so the cache-strategy must be set to 'content' to ensure that it is used during a bind-mount. And, instead of dirtying the worktree more with the cache file, I moved it to the .angular folder. This makes it easy to exclude the angular build cache and the eslint cache at once from a bind mount. In the default case, where the eslintcache is stored as a file in the root of the project, it's impossible to exclude a single file from a bind-mount while you can ignore whole folders. |
|
Hi @strawburster. I don't use containers for development but your approach sounds reasonable. |
Add eslint cache to docker image, even if there are linting errors. The cache is stored in the .angular/cache folder.
bb8516c to
f502881
Compare
|
I apologize, I tried to add the --force flag to the lint script in the dockerfile, but it wasn't passed correctly, I thought I had tested it but apparently not. I updated the original comment because the statistics changed after I changed the options. |
The container scripts were given to help review this PR, but the changes to the lint scripts in the package.json will affect all developers that lint locally, whether or not they use docker. |
|
Is the AuthService test flaky? I really doubt this change caused any meaningful difference in build output. |
@strawburster lots of GitHub CI runs have intermittent failures. Let's restart the job to see if it finishes this time. |
Description
Cache eslint runs so that they will run faster in the future using a cache file in the
./.angular/cachefolder.This PR was PR-ovided by The Library Code
Instructions for Reviewers
Build the angular development image and run the linter on it:
On my machine this is 0m3.840s
vs latest docker angular development build:
On my machine it is 1m25.251s
The image size is increased by 56.6 megabytes as a result of the cache being included, and the initial build time for the angular image will be increased by approximately 90 seconds.
List of changes in this PR:
Checklist
This checklist provides a reminder of what we are going to look for when reviewing your PR. You do not need to complete this checklist prior creating your PR (draft PRs are always welcome).
However, reviewers may request that you complete any actions in this list if you have not done so. If you are unsure about an item in the checklist, don't hesitate to ask. We're here to help!
mainbranch of code (unless it is a backport or is fixing an issue specific to an older branch).npm run lintnpm run check-circ-deps)package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.