Skip to content

Conversation

@czentgr
Copy link
Contributor

@czentgr czentgr commented Oct 29, 2025

The format checker is not called when yml files in the github workflow are modified but none in
presto-native-execution. Thus, skipping changes that are found to be incorrect when the format check is called on the next PR.

Description

Motivation and Context

Impact

Test Plan

Contributor checklist

  • Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
  • PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
  • Documented new properties (with its default value), SQL syntax, functions, or other functionality.
  • If release notes are required, they follow the release notes guidelines.
  • Adequate tests were added if applicable.
  • CI passed.
  • If adding new dependencies, verified they have an OpenSSF Scorecard score of 5.0 or higher (or obtained explicit TSC approval for lower scores).

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Oct 29, 2025
@sourcery-ai
Copy link
Contributor

sourcery-ai bot commented Oct 29, 2025

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Ensure the C++ format and header check workflow triggers on any GitHub workflow YAML change by broadening the path filter and correct minor formatting anomalies in the JDBC connector tests workflow file.

File-Level Changes

Change Details Files
Expanded format checker to cover all workflow YAML files
  • Changed pull_request paths filter from a specific workflow file to '.github/workflows/**'
  • Updated workflow trigger to detect edits in any workflow under .github/workflows
.github/workflows/prestocpp-format-and-header-check.yml
Fixed formatting in JDBC connector tests workflow
  • Added missing newline at end of file for formatting compliance
.github/workflows/jdbc-connector-tests.yml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@czentgr czentgr requested a review from tdcmeehan October 29, 2025 23:08
@czentgr czentgr marked this pull request as ready for review October 29, 2025 23:08
@czentgr czentgr requested review from a team and unidevel as code owners October 29, 2025 23:08
@prestodb-ci prestodb-ci requested review from a team, ScrapCodes and xin-zhang2 and removed request for a team October 29, 2025 23:08
@czentgr czentgr force-pushed the cz_fix_format_scan branch from 494fb46 to 7c47231 Compare October 29, 2025 23:09
@czentgr czentgr changed the title fix(build): Check format for workflow yml files fix(ci): Check format for workflow yml files Oct 29, 2025
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • Consider scoping the prestocpp-format-and-header-check trigger more narrowly than .github/workflows/** to avoid running the C++ format check on unrelated workflow changes.
  • Restore the missing newline at the end of jdbc-connector-tests.yml to maintain consistent file formatting.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider scoping the prestocpp-format-and-header-check trigger more narrowly than `.github/workflows/**` to avoid running the C++ format check on unrelated workflow changes.
- Restore the missing newline at the end of `jdbc-connector-tests.yml` to maintain consistent file formatting.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

The format checker is not called when yml files in
the github workflow are modified but none in
presto-native-execution. Thus, skipping changes that
are found to be incorrect when the format check is
called on the next PR.
@czentgr czentgr force-pushed the cz_fix_format_scan branch from 7c47231 to 80e99a4 Compare October 30, 2025 03:56
Copy link
Contributor

@unidevel unidevel left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@hantangwangd hantangwangd left a comment

Choose a reason for hiding this comment

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

Thanks @czentgr

@hantangwangd hantangwangd merged commit 694bfa3 into prestodb:master Oct 30, 2025
81 checks passed
czentgr added a commit that referenced this pull request Oct 30, 2025
An accidental change was made that change one of the value to persist
credentials from false to true.
This PR reverts this accidental change.

Introduced by: #26473

## Description
<!---Describe your changes in detail-->

## Motivation and Context
<!---Why is this change required? What problem does it solve?-->
<!---If it fixes an open issue, please link to the issue here.-->

## Impact
<!---Describe any public API or user-facing feature change or any
performance impact-->

## Test Plan
<!---Please fill in how you tested your change-->

## Contributor checklist

- [ ] Please make sure your submission complies with our [contributing
guide](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md),
in particular [code
style](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#code-style)
and [commit
standards](https://github.com/prestodb/presto/blob/master/CONTRIBUTING.md#commit-standards).
- [ ] PR description addresses the issue accurately and concisely. If
the change is non-trivial, a GitHub Issue is referenced.
- [ ] Documented new properties (with its default value), SQL syntax,
functions, or other functionality.
- [ ] If release notes are required, they follow the [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines).
- [ ] Adequate tests were added if applicable.
- [ ] CI passed.
- [ ] If adding new dependencies, verified they have an [OpenSSF
Scorecard](https://securityscorecards.dev/#the-checks) score of 5.0 or
higher (or obtained explicit TSC approval for lower scores).

## Release Notes
Please follow [release notes
guidelines](https://github.com/prestodb/presto/wiki/Release-Notes-Guidelines)
and fill in the release notes below.

```
== NO RELEASE NOTE ==
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:IBM PR from IBM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants