Skip to content

Conversation

tobiasdiez
Copy link
Contributor

📚 Description

Related to #35522 and #35544. Should at least fix the changed codecov between runs.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@mkoeppe
Copy link
Contributor

mkoeppe commented May 9, 2023

I think this is a good solution. The goal of #29935 is still sufficiently served by users' local tests and but other workflow runs (such as the ci-linux runs on release tags). @kliem any objections?

@tobiasdiez
Copy link
Contributor Author

There were also two runs for the push on the dev branch (one for the push, and one for the tag), so I guess we will only see after merging this if it really works then.

@kliem
Copy link
Contributor

kliem commented May 9, 2023

I think this is a good solution. The goal of #29935 is still sufficiently served by users' local tests and but other workflow runs (such as the ci-linux runs on release tags). @kliem any objections?

I think the suggested changes are an improvement.

Overall, I still think that if people write a test that claims that something works for any random input of a type, it should. E.g. matrix addition should be entry-wise addition.

However, when I pushed for enforcing the random seed for CI, we still used patchbots that would occasionally fail anyway. Now, we use a more stable CI, I believe. If we are able to eliminate almost all noise from the CI, we should I think. And it's also good, if you don't get affected by problems not related to your PR.

@github-actions
Copy link

github-actions bot commented May 9, 2023

Documentation preview for this PR is ready! 🎉
Built with commit: aba447a

@@ -3,6 +3,9 @@ name: Build & Test
on:
pull_request:
push:
branches: ['**']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just replace this by

push: 
  branches:

(it won't run on tags then)

@vbraun vbraun merged commit a80034a into sagemath:develop May 28, 2023
@tobiasdiez tobiasdiez deleted the rand branch May 28, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants