Skip to content

Conversation

@kingthorin
Copy link
Collaborator

Similar to the recent auth-tests addition.

@thc202
Copy link
Collaborator

thc202 commented May 21, 2025

This will not work as intended, the step # Update to the latest upstream needs to be changed as well, pushing the same content as the upstream is what closes the PR.

I'd also prefer that we don't fail the workflow when the PR is just being updated, there's no reason to and just creates noise (still getting one each day).

@kingthorin
Copy link
Collaborator Author

I'll look at the first point.
For the failures:

          gh pr view --json mergedAt|jq -e '.mergedAt' && gh pr create --fill
          gh pr view --json mergedAt|jq -e '.mergedAt' && gh pr create --fill && true

(or ;; true)

@kingthorin
Copy link
Collaborator Author

I updated the juiceshop workflow, does that address your first concern?

@kingthorin kingthorin force-pushed the pr-update branch 2 times, most recently from 6d6604a to 9e5b266 Compare May 21, 2025 12:54
@thc202
Copy link
Collaborator

thc202 commented May 26, 2025

Seems so, it's no longer pushing the latest from upstream.

@kingthorin
Copy link
Collaborator Author

And which option for the failures or do you have another idea?

@thc202
Copy link
Collaborator

thc202 commented May 27, 2025

I don't mind which way as long as it only fails if there's an actual failure (to create the PR or push the changes).

@kingthorin
Copy link
Collaborator Author

In that case it would require something more like exit code logic, but I'm not sure they get specific enough.

For the time being, let's narrow this PR down to adjusting the PR logic in other workflows. Then I can look at the failure logic in another?

@thc202
Copy link
Collaborator

thc202 commented Jun 4, 2025

I'm fine with that, there aren't other ongoing PRs so failures should be minimal.

@kingthorin
Copy link
Collaborator Author

All adjusted.

@kingthorin kingthorin mentioned this pull request Aug 8, 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.

2 participants