Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the PR description for auto webassets udpates #10212

Merged
merged 1 commit into from
Feb 8, 2022

Conversation

zmb3
Copy link
Collaborator

@zmb3 zmb3 commented Feb 8, 2022

The script for updating webassets uses the commit message from
webapps as the commit message for the PR to teleport.

This commit message is almost always a merged PR, which has the format:

do some awesome thing (#123)

Where #123 is the number of the webapps PR that was merged.

The problem with this is, when the teleport PR is created, it interprets
the #123 as the number of a teleport PR. And since the Teleport repo
has a lot more issues/PRs than webapps, Github ends up linking to an old
and completely unrelated PR.

Fix this by replacing (#123) with (gravitational/webapps#123), which
Github correctly renders as a link to the webapps PR in question.

@github-actions github-actions bot requested review from nklaassen and Tener February 8, 2022 01:57
@Tener
Copy link
Contributor

Tener commented Feb 8, 2022

LGTM, except for the fact that the script could use a comment on what are we doing with sed, as pointed out by @hatched .

@zmb3 zmb3 force-pushed the zmb3/fix-backport-script-pr-link branch from 9ad6b85 to 0f7d2a9 Compare February 8, 2022 15:30
Copy link
Contributor

@ibeckermayer ibeckermayer left a comment

Choose a reason for hiding this comment

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

LGTM

image

Copy link
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

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

thanks for fixing this! 🚀

@zmb3 zmb3 force-pushed the zmb3/fix-backport-script-pr-link branch from 0f7d2a9 to fb05cda Compare February 8, 2022 18:35
@zmb3 zmb3 enabled auto-merge (squash) February 8, 2022 18:38
The script for updating webassets uses the commit message from
webapps as the commit message for the PR to teleport.

This commit message is almost always a merged PR, which has the format:

    do some awesome thing (#123)

Where '#123' is the number of the **webapps** PR that was merged.

The problem with this is, when the teleport PR is created, it interprets
the #123 as the number of a **teleport** PR. And since the Teleport repo
has a lot more issues/PRs than webapps, Github ends up linking to an old
and completely unrelated PR.

Fix this by replacing (#123) with (gravitational/webapps#123), which
Github correctly renders as a link to the webapps PR in question.
@zmb3 zmb3 force-pushed the zmb3/fix-backport-script-pr-link branch from fb05cda to 5ac7f03 Compare February 8, 2022 18:51
@zmb3 zmb3 merged commit b2ffe8c into master Feb 8, 2022
@zmb3 zmb3 deleted the zmb3/fix-backport-script-pr-link branch February 8, 2022 19:10
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.

6 participants