You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "comphead (via GitHub)" <gi...@apache.org> on 2025/08/11 20:21:22 UTC
[PR] chore: Enforce checks for RC branches [datafusion]
comphead opened a new pull request, #17132:
URL: https://github.com/apache/datafusion/pull/17132
## Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
- Closes #.
## Rationale for this change
This PR should close couple of pending issues with release candidate branches:
- Protect release candidate branch with same reviewer policy as the `main` branch to prevent branch to be ruined by unexpected commit/merge
- Run `extended` tests as part of RC CI, it is expected to have few number of hotfixes so it would be acceptable to run tests per PR
- update documentation to modify release number in `asf.yaml` because the Github doesn't support wildcards, only exact branch names
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
## What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
## Are these changes tested?
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code
If tests are not included in your PR, please explain why (for example, are they covered by existing tests)?
-->
## Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!--
If there are any breaking changes to public APIs, please add the `api change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2268094978
##########
dev/release/README.md:
##########
@@ -36,8 +36,8 @@ If you would like to propose your change for inclusion in a release branch for a
patch release:
1. Find (or create) the issue for the incremental release ([example release issue]) and discuss the proposed change there with the maintainers.
-1. Follow normal workflow to create PR to `main` branch and wait for its approval and merge.
-1. After PR is squash merged to `main`, branch from most recent release branch (e.g. `branch-37`), cherry-pick the commit and create a PR targeting the release branch [example backport PR].
+2. Follow normal workflow to create PR to `main` branch and wait for its approval and merge.
+3. After PR is squash merged to `main`, branch from most recent release branch (e.g. `branch-37`), cherry-pick the commit and create a PR targeting the release branch [example backport PR].
Review Comment:
markdown is smart to render bullet numbers no matter what is in the MD code, but when you read the MD code it is confusing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2268101959
##########
dev/release/README.md:
##########
@@ -128,6 +128,15 @@ release.
See [#9697](https://github.com/apache/datafusion/pull/9697) for an example.
+Modify `asf.yaml` to protect future release candidate branch to prevent accidental merges:
+
+```yaml
+# needs to be updated as part of the release process
+branch-50:
Review Comment:
> also, you sure patterns don't work? at least in branch protection rules they are actually working
Github UI supports the wildcard, but `asf.yaml` surprisingly does not https://github.com/apache/infrastructure-asfyaml?tab=readme-ov-file#branch-protection
> I think hardcoding with a comment stating why we do that is reasonable. We can also do 10 or 15 years which IMO is probably enough, 100 is overkill.
Agree, lets have 10-20 branches hardcoded.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead merged PR #17132:
URL: https://github.com/apache/datafusion/pull/17132
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2268078656
##########
dev/release/README.md:
##########
@@ -128,6 +128,15 @@ release.
See [#9697](https://github.com/apache/datafusion/pull/9697) for an example.
+Modify `asf.yaml` to protect future release candidate branch to prevent accidental merges:
+
+```yaml
+# needs to be updated as part of the release process
+branch-50:
Review Comment:
yeah, I was thinking about it for some time and dont have strong opinion on that. The file would be bloated for sure, yaml doesn't support `import`. Another option is to create an automation script that modifies the `asf.yaml` as part of prerelease preps but it needs to be tested.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#issuecomment-3176783124
@alamb @findepi @andygrove FYI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2269300595
##########
.asf.yaml:
##########
@@ -50,6 +50,74 @@ github:
main:
required_pull_request_reviews:
required_approving_review_count: 1
+ # needs to be updated as part of the release process
+ # Github doesn't support wildcard branch protection rules, only exact branch names
Review Comment:
I believe GitHub does support that. At least I have set wildcard-based protection rules in the past. Might it be that ASF automation does not?
##########
.github/workflows/extended.yml:
##########
@@ -32,6 +32,8 @@ on:
push:
branches:
- main
+ # release candidate branches
+ - 'branch-*'
Review Comment:
There are also `github.ref_name == 'main'` checks in the rust workflow.
Currently they look harmless, as they only conditionally enable "some cache" (no sure what that cache is for...).
Maybe worth adding a code comment in these usages to make it less likely someone will clopy-paste into a more important job step?
##########
.github/workflows/extended.yml:
##########
@@ -32,6 +32,8 @@ on:
push:
branches:
- main
+ # release candidate branches
+ - 'branch-*'
Review Comment:
Same in docs & audit workflows?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2268083254
##########
dev/release/README.md:
##########
@@ -128,6 +128,15 @@ release.
See [#9697](https://github.com/apache/datafusion/pull/9697) for an example.
+Modify `asf.yaml` to protect future release candidate branch to prevent accidental merges:
+
+```yaml
+# needs to be updated as part of the release process
+branch-50:
Review Comment:
we don't work with that yaml that often so maybe big file is fine?
also, you sure patterns don't work? at least in branch protection rules they are actually working
<img width="946" height="409" alt="image" src="https://github.com/user-attachments/assets/aab313ae-592f-41ac-93bd-56e3340f2f3b" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2270283305
##########
.asf.yaml:
##########
@@ -50,6 +50,74 @@ github:
main:
required_pull_request_reviews:
required_approving_review_count: 1
+ # needs to be updated as part of the release process
+ # Github doesn't support wildcard branch protection rules, only exact branch names
Review Comment:
Appreciate if you can point me to that doc. What I understood from https://github.com/apache/infrastructure-asfyaml?tab=readme-ov-file#branch-protection for `asf.yml` it is exact name support
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2268084291
##########
dev/release/README.md:
##########
@@ -128,6 +128,15 @@ release.
See [#9697](https://github.com/apache/datafusion/pull/9697) for an example.
+Modify `asf.yaml` to protect future release candidate branch to prevent accidental merges:
+
+```yaml
+# needs to be updated as part of the release process
+branch-50:
Review Comment:
I think hardcoding with a comment stating why we do that is reasonable. We can also do 10 or 15 years which IMO is probably enough, 100 is overkill.
##########
dev/release/README.md:
##########
@@ -36,8 +36,8 @@ If you would like to propose your change for inclusion in a release branch for a
patch release:
1. Find (or create) the issue for the incremental release ([example release issue]) and discuss the proposed change there with the maintainers.
-1. Follow normal workflow to create PR to `main` branch and wait for its approval and merge.
-1. After PR is squash merged to `main`, branch from most recent release branch (e.g. `branch-37`), cherry-pick the commit and create a PR targeting the release branch [example backport PR].
+2. Follow normal workflow to create PR to `main` branch and wait for its approval and merge.
+3. After PR is squash merged to `main`, branch from most recent release branch (e.g. `branch-37`), cherry-pick the commit and create a PR targeting the release branch [example backport PR].
Review Comment:
Curious why this update? Most markdown rendering engines will auto-number if you use the existing approach. E.g. when viewed on GitHub the existing version is correctly numbered.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2267994251
##########
dev/release/README.md:
##########
@@ -128,6 +128,15 @@ release.
See [#9697](https://github.com/apache/datafusion/pull/9697) for an example.
+Modify `asf.yaml` to protect future release candidate branch to prevent accidental merges:
+
+```yaml
+# needs to be updated as part of the release process
+branch-50:
Review Comment:
we can also create rules for versions 50..150 and be safe for the next 10 years 😁
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] chore: Enforce checks for RC branches [datafusion]
Posted by "findepi (via GitHub)" <gi...@apache.org>.
findepi commented on code in PR #17132:
URL: https://github.com/apache/datafusion/pull/17132#discussion_r2271130926
##########
.asf.yaml:
##########
@@ -50,6 +50,74 @@ github:
main:
required_pull_request_reviews:
required_approving_review_count: 1
+ # needs to be updated as part of the release process
+ # Github doesn't support wildcard branch protection rules, only exact branch names
Review Comment:
GH
<img width="1164" height="751" alt="image" src="https://github.com/user-attachments/assets/d273d7b9-f45f-4c65-949b-304a7fab3a3a" />
ASF.yml -- i don't know anything about what it supports
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org