Skip to content

Conversation

@assignUser
Copy link
Member

@assignUser assignUser commented Mar 16, 2025

Rationale for this change

See: https://lists.apache.org/thread/wxtcq0xtfcr2rrdlowo93qqvpvhb10pl

What changes are included in this PR?

  • turn on discussions
  • turn on existing features
    • this is required as the recently updated backend of .asf.yaml will now turn features of that have previously been turned on manually and are not part of the current config
  • protect main against force pushes
    • this is imo a basic security feature. any additional rules would need to work with our current release process that currently requires direct pushes to main

Are these changes tested?

The asf yaml parser will send an email if the config is broken, this only works when the change is made from within the repo and not a fork.

Are there any user-facing changes?

Yes, discussions are now available.

@assignUser assignUser requested review from kou and raulcd as code owners March 16, 2025 17:16
@github-actions github-actions bot added the awaiting review Awaiting review label Mar 16, 2025
@kou
Copy link
Member

kou commented Mar 16, 2025

Let's open an issue for better visibility an openness. This doesn't change any features but this is a change that affects users and developers.

Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

Agree with @kou, let's create an issue to track this.

@github-actions github-actions bot added awaiting review Awaiting review awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Mar 17, 2025
@raulcd
Copy link
Member

raulcd commented Mar 17, 2025

Also, it seems you pushed your branch to the apache/arrow remote instead of your fork :)

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review awaiting changes Awaiting changes labels Mar 17, 2025
@assignUser
Copy link
Member Author

The asf yaml parser will send an email if the config is broken, this only works when the change is made from within the repo and not a fork.

This caught my typo with users@

An error occurred while processing the notifications feature in .asf.yaml:

[ERROR] The mailing list target, [[email protected]](mailto:[email protected]), set in notifications::discussions, is not an existing ASF mailing list.

@assignUser assignUser changed the title MINOR: [Docs] Enable discussions GH-45813: [Docs] Enable discussions Mar 17, 2025
@github-actions
Copy link

⚠️ GitHub issue #45813 has been automatically assigned in GitHub to PR creator.

Copy link
Member

@raulcd raulcd left a comment

Choose a reason for hiding this comment

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

I'll merge this now that the vote has passed

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Mar 24, 2025
@raulcd
Copy link
Member

raulcd commented Mar 24, 2025

@raulcd raulcd merged commit 9c41cdc into main Mar 24, 2025
14 checks passed
@raulcd raulcd removed the awaiting merge Awaiting merge label Mar 24, 2025
@raulcd raulcd deleted the enable-discussions branch March 24, 2025 09:11
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 9c41cdc.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

zanmato1984 pushed a commit to zanmato1984/arrow that referenced this pull request Apr 15, 2025
### Rationale for this change
See: https://lists.apache.org/thread/wxtcq0xtfcr2rrdlowo93qqvpvhb10pl

### What changes are included in this PR?
- turn on discussions
- turn on existing features
  - this is required as the recently updated backend of .asf.yaml will now turn features of that have previously been turned on manually and are not part of the current config
- protect main against force pushes 
  - this is imo a basic security feature. any additional rules would need to work with our current release process that currently requires direct pushes to main

### Are these changes tested?
The asf yaml parser will send an email if the config is broken, this only works when the change is made from within the repo and not a fork.

### Are there any user-facing changes?
Yes, discussions are now available.
* GitHub Issue: apache#45813

Authored-by: Jacob Wujciak-Jens <[email protected]>
Signed-off-by: Raúl Cumplido <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants