Skip to content

Conversation

@cgundy
Copy link
Member

@cgundy cgundy commented Dec 10, 2024

Motivation

This updates the comment with the currently correct information.

Changes

No code changes, just updating a comment.

@cgundy cgundy marked this pull request as ready for review December 10, 2024 13:15
@cgundy cgundy requested review from a team as code owners December 10, 2024 13:15
# changes and instead just fail if the formatting is incorrect.
# In order to trigger other workflows after committing docs changes, we need
# to use the PR Automation App. This token is not available for external
# contributors. So on forked PRs, we don't commit changes and instead just fail
Copy link
Contributor

Choose a reason for hiding this comment

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

By "forked PR", do you mean a PR created from a fork of the repo?
We used to specifically check if the PR came from a fork of the repo, but this wasn't good enough as discussed here, so we changed it in this PR to just check for the presence of the token instead.

But now you are changing the comment back from being about the presence of the token to about being forked. Do the previous considerations no longer apply?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah wow, good memory! Should i just be very explicit then and say "forked PR or from dependabot"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is dependabot really the only other possibility?
Isn't there another way to do what dependabot does?
Why not just stick with "PRs where the app secret is not available"?

Even though I guessed correctly what was meant by "forked PR", I do find that phrase a bit confusing.

# changes and instead just fail if the formatting is incorrect.
# In order to trigger other workflows after committing docs changes, we need
# to use the PR Automation App. This token is not available for external
# contributors. So on forked PRs, we don't commit changes and instead just fail
Copy link
Contributor

Choose a reason for hiding this comment

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

Is dependabot really the only other possibility?
Isn't there another way to do what dependabot does?
Why not just stick with "PRs where the app secret is not available"?

Even though I guessed correctly what was meant by "forked PR", I do find that phrase a bit confusing.

@cgundy cgundy enabled auto-merge (squash) December 10, 2024 17:32
@cgundy cgundy merged commit fc6a30d into main Dec 10, 2024
8 checks passed
@cgundy cgundy deleted the update-comment branch December 10, 2024 17:32
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