- Remove temporary apt files
- Removes LLVM16 TODO
- Group Reviewers
- rGd22cab2e11f4: [libc++][CI] Improves Dockerfile
I really don't see how describing what the lines do on-by-one help in any way. They just make it harder to read IMO.
I like that we are removing the LLVM 16 todo, but I am not sold on the other changes to the Dockerfile. I'd like to understand your rationale for them.
Isn't it better to have more layers? That way you get more layers of caching and can potentially re-download less when you update the image, no?
As discussed, I am not sure this actually solves a problem we encounter -- our image isn't that large and I don't think the image download times have been a problem. If that's incorrect, please LMK since that changes my opinion about this patch.
But assuming that we're not really gaining something noticeable in terms of speed/size, IMO this patch is a small net loss since it's a bit less readable than what we had before. This is a part of our CI infrastructure and IMO we should strive to keep it as simple as possible to keep the barrier for people maintaining it as low as possible -- to me that has more value than a bit more size in the image.
FWIW I'd be fine with cleaning up the Debian packages in a separate RUN command. This won't reduce the size we have to download, but would make the image itself a bit smaller, I guess.
Thanks for the review. I will basically revert these changes and only remove the unneeded package and the apt removal.
As discussed with Louis this entire hunk will be reverted.