Hi, On 2021-11-19 17:17:44 -0500, Melanie Plageman wrote: > For the README, I found the instructions very clear. My only concern is > that the cirrus-ci UI will change and the instructions on how to enable > cirrus-ci on a repository will not be accessible in the same way in the > future.
I think we can just adjust things at that point, I'm not too worried about past instructions not working. > I have attached a patch which does a small refactor using a yaml anchor > and aliases (tried it and it seems to work for me). Oh, neat. Yaml is so weird. > - Would you find it valuable to set a few more coredump_filter bits? > Might be worth setting bits 2 and 3 (see [2])-- in addition to the > defaults (on Linux -- I don't know what the equivalent is on other > platforms). I don't think we need 2/3 - we don't have file backed mappings. In some situations setting bit 6 (shared huge pages) would make sense - but here we don't configure them... > - I found this line a bit confusing, so maybe it is worth a comment > sysinfo_script: > - export || true We can probably just get rid of the ||. It was just so that a missing 'export' builtin didn't cause execution to abort. But that won't randomly vanish, so it's fine. > - For the docker files, I think it is recommended to run "docker build" > only from within the specific build context (not in the top-level > directory), so I don't think you should need the dockerignore file. We don't have control over that in this case - it's cirrus invoking docker, and it just uses the whole repo as the context. > - In ci/docker/linux_debian_bullseye, you can make this change: > - apt-get clean > + apt-get clean && \ > + rm -f /var/lib/apt/lists/* Might not matter too much compared to the size of the whole thing, but it definitely won't hurt... Greetings, Andres Freund