-
-
Notifications
You must be signed in to change notification settings - Fork 481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reduce number of systems test with minimal config #36941
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks
This is just vandalism. |
no, your minimal configurations are vandalism. |
@dimpase How so? |
cause it's vandalism to test on CI 1000s times something big and stable, like building a particular version of gcc/gfortran, on the same systems - on which this gcc/gfortran is absolutely not needed. This is basically burning money. |
What you are describing does not fit the definition of "vandalism" at all. I'll take it as a reflexive reaction, a "no, you are" comeback. |
well, pick up your favourite term for doing meaningless, time-consuming, and potentially expensive stuff, and forcing other people to help you with it. |
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Alternative solution for sagemath#36726. Since the purpose of the "minimal" ci runs is to check the documentation of the minimal build requirements, it is overkill to run it for all systems. Thus, we only run it for "ubuntu- focal", which is also the default system for PRs. This also hugely reduces the load of onto the ci, somewhat leviating the issues introduced by sagemath#36616 (see eg sagemath#36616 (comment)). <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36941 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik
@vbraun Please back this out |
@dimpase I called you out clearly for making such denunciations, which are clear CoC violations. #36726 (comment) |
@mkoeppe - have you read @jhpalmieri 's email? |
@dimpase Yes. How do you justify your continued severe misconduct? |
<!-- ^^^^^ Please provide a concise, informative and self-explanatory title. Don't put issue numbers in there, do this in the PR body below. For example, instead of "Fixes sagemath#1234" use "Introduce new method to calculate 1+1" --> <!-- Describe your changes here in detail --> Alternative solution for sagemath#36726. Since the purpose of the "minimal" ci runs is to check the documentation of the minimal build requirements, it is overkill to run it for all systems. Thus, we only run it for "ubuntu- focal", which is also the default system for PRs. This also hugely reduces the load of onto the ci, somewhat leviating the issues introduced by sagemath#36616 (see eg sagemath#36616 (comment)). <!-- Why is this change required? What problem does it solve? --> <!-- If this PR resolves an open issue, please link to it here. For example "Fixes sagemath#12345". --> <!-- If your change requires a documentation PR, please link it appropriately. --> ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> <!-- If your change requires a documentation PR, please link it appropriately --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> <!-- Feel free to remove irrelevant items. --> - [x] The title is concise, informative, and self-explanatory. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation accordingly. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on - sagemath#12345: short description why this is a dependency - sagemath#34567: ... --> <!-- If you're unsure about any of these, don't hesitate to ask. We're here to help! --> URL: sagemath#36941 Reported by: Tobias Diez Reviewer(s): Dima Pasechnik
Documentation preview for this PR (built with commit 72b72cd; changes) is ready! 🎉 |
This PR is an appropriate reaction to #36726. It removes CI testing for all Linux distros except one version of Ubuntu (which happens to have pkill). It is an extreme solution to a problem that is already solved. Reducing testing to only one distro is something that should obviously be discussed and voted on sage-devel. A civil discussion on sage-devel of the pros and cons of different approaches to CI testing of sage would be valuable; this is not. |
Alternative solution for #36726. Since the purpose of the "minimal" ci runs is to check the documentation of the minimal build requirements, it is overkill to run it for all systems. Thus, we only run it for "ubuntu-focal", which is also the default system for PRs.
This also hugely reduces the load of onto the ci, somewhat leviating the issues introduced by #36616 (see eg #36616 (comment)).
📝 Checklist
⌛ Dependencies