-
Notifications
You must be signed in to change notification settings - Fork 56
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
Document casing of C implementations of SQL functions #230
Conversation
This is to make it easy to grep for these functions.
Ugh... Why is skipping builds so hard if you also want to have branch-protections... : https://www.pantsbuild.org/blog/2022/10/10/skipping-github-actions-jobs-without-breaking-branch-protection |
Mmmm, more CI hackery. We could turn off the path restrictions given the CI is quite quick now? That said... while I'm certainly not opposed to the branch protection rules, if they're just causing problems, maybe we don't need them. Personally, I'm a fan of setting rules and trust people will do the right thing, rather than enforce the rules in software. In this case, that would mean no branch protection rules [for CI], but trust that committers will only merge if the tests pass. I don't think we've ever merged without tests passing, so we're not really solving a problem with the branch protection rules. But... the auto-merge feature is nice. |
Yeah, I think that's probably the most pragmatic solution. In ~95% of the PRs we change some of these paths anyway. Really only doc-only PRs benefit from not running CI.
Agreed
Yeah, this is the main reason I would like to keep the branch protection rules. If we could have auto-merge work without them, I'd be all for not having required checks in the branch protection. |
We were skipping CI when certain files were not touched. This is prone to causing problems. 1. Because it's an include and not exclude list, we need to be careful to add files to it. This caused some confusing in #98 2. Branch protection rules and this skipping work terribly together, because the branch protection rules will not allow merging the PR unless the job was actually run. We ran into this in #230. There are ways to work around this, such as described in the following blogpost, but it's quite a bit of hackery: <https://www.pantsbuild.org/blog/2022/10/10/skipping-github-actions-jobs-without-breaking-branch-protection> So the most pragmatic solution seems to be not to skip CI anymore. CI is now pretty quick so the benefit of skipping it is already fairly small, and the number of PRs where we can actually skip CI is also really limited (basically only for docs-only PRs).
This is to make it easy to grep for these functions.