search

rust-itertools / itertools

Extra iterator adaptors, iterator methods, free functions, and macros.
https://docs.rs/itertools/
Apache License 2.0
2.72k stars 309 forks source link

Guaranteeing health of the default branch #769

Closed warren2k closed 9 months ago

warren2k commented 1 year ago

We just reported #768. How did failing code reach the default branch?

mightyiam commented 1 year ago

As was mentioned in https://github.com/rust-itertools/itertools/issues/768#issuecomment-1741780757, there is no attempt to run cargo test --all-targets in CI. Hey, I don't even know exactly what that even means. Apparently, it means running benchmarks, as well.

But... it wouldn't hurt, I suppose, to run the CI workflow on push to master, as well. As a confirmation that whatever did reach the default branch, is, in fact, healthy, just in case.

Philippe-Cholet commented 1 year ago

I just fixed the reported error in #770 (see details there). It was nearly nothing and definitely not an issue because benchmarks only run with release profile. I don't think we need to do anything more. This was surprising, that's for sure.

Philippe-Cholet commented 1 year ago

@warren2k @mightyiam Do you think we need to do anything more or should we close this? Since a few days (more recent than this issue), we are using a "merge queue" which from my understanding of it is a bit better to guarantee the health of the default branch.

mightyiam commented 1 year ago

We never looked into the merge queue feature. We'll leave this decision up to you. Feel free to close the issue at your discretion.

Philippe-Cholet commented 9 months ago

Guarantee the health of the default branch is not a destination but a journey that we won't forget so I don't think we need this issue as a remainder (so I'm closing this). Plus, CI has been quite improved lately, thanks to you both.