Closed abelsiqueira closed 2 months ago
I suggest dropping Cirrus CI @tmigot, what do you think?
I suggest dropping Cirrus CI @tmigot, what do you think?
I personally agree. What do you think @amontoison ?
@tmigot I opened #166 to fix CirrusCI. Can you rebase this branch?
Thanks, @amontoison. I still think we should delete Cirrus CI testing from this package. I don't think the main use case of this package requires testing on FreeBSD or a specific Mac that GH doesn't already provide. This is essentially an interface, so the underlying packages should be more thoroughly tested, but this doesn't need to. @tmigot, let me know if you still agree, I'll open an issue then. I will rebase this.
The issue was that MacOS silicon was not supported by GHA until a very recent update.
I think that Cirrus is relevant for packages that depend on Artifact (JLL packages) but we can remove it otherwise.
The issue was that MacOS silicon was not supported by GHA until a very recent update.
I think that Cirrus is relevant for packages that depend on Artifact (JLL packages) but we can remove it otherwise.
Thanks! So, still good to me to remove cirrus for this package.
The issue was that MacOS silicon was not supported by GHA until a very recent update. I think that Cirrus is relevant for packages that depend on Artifact (JLL packages) but we can remove it otherwise.
Thanks! So, still good to me to remove cirrus for this package.
Yes, we just need to update GHA to test on two architectures (x64 and aarch64). macos-13 is the last release of Mac that supports this architecture. macos-lastest is only working on aarch64 now.
I've updated the PR with the latest version of the template. I've removed CirrusCI. If we apply the template to other packages in JSO that use CirrusCI, we can update the template with the latest file.
As for the extra tests, I also don't see the benefit of adding them now, in this package.
The Lint failures are due to broken links. After merging, most or all should be fixed.
I've removed CirrusCI.
Why?
Thanks, @amontoison. I still think we should delete Cirrus CI testing from this package. I don't think the main use case of this package requires testing on FreeBSD or a specific Mac that GH doesn't already provide. This is essentially an interface, so the underlying packages should be more thoroughly tested, but this doesn't need to. @tmigot, let me know if you still agree, I'll open an issue then. I will rebase this.
@dpo, ☝️
The issue was that MacOS silicon was not supported by GHA until a very recent update.
I think that Cirrus is relevant for packages that depend on Artifact (JLL packages) but we can remove it otherwise.
With that logic, there's no need to test on Windows or macOS either. I disagree. Do the FreeBSD tests not pass? The more platforms we cover, the more robust our code.
I think the cost-benefit of adding FreeBSD is not good enough, but it is for Mac and Windows. That being said, if it makes it easier to adopt COPIERTemplate, I'm glad to reintroduce it. The file in the template is the following: https://github.com/abelsiqueira/COPIERTemplate.jl/blob/main/template/%7B%25%20if%20UseCirrusCI%20%25%7D.cirrus.yml%7B%25%20endif%20%25%7D.jinja I've added the file there because JSO uses Cirrus CI, so can you update that file to something that makes sense for JSO? I can also remove from there so it is used directly here.
My arguments for not having FreeBSD in this package are:
@dpo @tmigot can we have a conclusion on this?
I just find it arbitrary to remove Cirrus from here while we use it everywhere else. I've found issues with code before because errors were triggered on a Cirrus VM. If we can cover another OS, why not? Predicting the effects of a change in one of the dependent packages is very hard, so the more coverage, the better, in my opinion.
@dpo can you fit in your schedule maintaining the template's Cirrus file?
Sure. there's really nothing to it.
Great, I'll update this then
I've updated to BestieTemplate (it has been renamed) version 0.7.1. The Cirrus CI is failing right now, so I don't know how you want to proceed @dpo
CirrusCI changed the rules recently and we only have a (very) limited number of credits now. We should drop it after this update
If we're dropping it, it should be here so we can squash merge this, and because it is one of the question of the template and if we drop it we simplify the application more.
@amontoison can you explain more on the changes or the limitation? @dpo had arguments to maintain, but if the limitation is too severe I imagine it might affect using Cirrus CI on other packages as well. Here's the original message:
I just find it arbitrary to remove Cirrus from here while we use it everywhere else. I've found issues with code before because errors were triggered on a Cirrus VM. If we can cover another OS, why not? Predicting the effects of a change in one of the dependent packages is very hard, so the more coverage, the better, in my opinion.
CirrusCI gives us 10,000 minutes (= 7 x 24 hours) per month for Linux VMs and 500 minutes for macOS VMs [1]. Is that such a limitation??? I'm a bit bummed by these discussions.
The failure in this PR is unrelated to time limits.
Can you submit a RP to update https://github.com/abelsiqueira/BestieTemplate.jl/? Otherwise it defeats the purpose of having Cirrus it on the template.
I've updated to 0.7.2 which includes @dpo's changes for Cirrus, and I merged @tmigot's suggestion, so it is done and I will merge now. Thanks everyone for their time and input.
Applied the template and made some manual changes, such as changing the docs names, fixing the README and removing some older workflows.
✅ Closes: #143