Skip to content

Conversation

@shivghai
Copy link
Contributor

Currently, we require the REPORT_COMMIT command to be triggered on the pull_request command. This is not necessary, since the only blocker here should be for us to have the pull_request payload in the GitHub Event

@shivghai shivghai requested a review from a team as a code owner March 13, 2025 12:30
@shivghai shivghai requested review from willhbang and zwaxman March 13, 2025 12:30
Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@shivghai shivghai force-pushed the 03-13-require-pull-request-body-not-event branch from 3a1abbb to 634bb14 Compare March 13, 2025 12:44
@zwaxman
Copy link
Contributor

zwaxman commented Mar 14, 2025

Wait actually do we want to do this? Don't we want them to start reporting on their merges (what I'm working on now). Do they run their pipeline on the pull_request event "merge" action (I think this is less commonly the case), or on a "push" event to main. They're not already running reportCommit on merge, are they? I think we need to double check with Vinko about this

Copy link
Contributor

@zwaxman zwaxman left a comment

Choose a reason for hiding this comment

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

Think we need to double check with Vinko how they will trigger their merge pipelines. I think on a "push" event to main is much more common pattern than to run on the pull_request event "merge" action.

@zwaxman
Copy link
Contributor

zwaxman commented Mar 14, 2025

Or I guess for our sakes, we could just require they run on the pull_request "merge" that's definitely easier for us to implement, I just think that's a less common pattern, so don't know if we'll get pushback from other clients here.

@shivghai shivghai closed this Mar 18, 2025
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