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

[NFC] Delete release branch PR template in favor of org-wide variant #940

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AnthonyLatsis
Copy link
Contributor

@AnthonyLatsis AnthonyLatsis commented Jun 4, 2024

Summary

swiftlang now has an equivalent organization-wide PR template, so we no longer need this local one. Also, prompt folks to use this template when targeting a release branch in the default PR template.

Checklist

This change is NFC.

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I'm -1 on this.

The originally markdown file gives us a nice template to start with.

But I agree the idea of centering the template across Swift project.

Copy link
Contributor Author

@AnthonyLatsis AnthonyLatsis Jun 18, 2024

Choose a reason for hiding this comment

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

I know, it sucks, but we believe one more layer of indirection is an acceptable compromise — for now. You can follow the link to the form directly from the default PR template too, no need to manually switch to this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I ignored the fact that "This move was agreed upon by the Contributor Experience Workgroup."

By the way, do we have some sort of Contributor Experience Workgroup Meeting notes like other workgroup? (eg. SDWG / SSWG / SWWG have it published on the forum)

I'm not doubt the authenticity, just want to know more relevant context here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. It'd be great to have a link to the meeting notes to understand more about what problem this is solving and who participated in that discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By the way, do we have some sort of Contributor Experience Workgroup Meeting notes like other workgroup?

No, we don’t have a public meeting record. I will put this on the agenda for next meeting and let you know how it went.

cc @xedin

Copy link
Contributor Author

@AnthonyLatsis AnthonyLatsis Jun 18, 2024

Choose a reason for hiding this comment

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

It'd be great to have a link to the meeting notes to understand more about what problem

To expand on the PR description, the problem is that different repositories have their own versions of the release branch PR form, I suppose mainly because no one updates them, and then there is yet another version that gets published on the forums with every release process announcement topic. These topics also suggest that the form must be central, and we are trying to clean things up and establish a open source place for it to live in, such that repositories can refer to it rather than failing at maintaining copies, while people can propose changes to it through pull requests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. It'd be great to have a link to the meeting notes to understand more about what problem this is solving and who participated in that discussion.

Copy link
Contributor

@Kyle-Ye Kyle-Ye left a comment

Choose a reason for hiding this comment

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

Instead of using swiftlang/swift-org-website#688

I'd like to suggest we add a new .github repo on swiftlang and add PULL_REQUEST_TEMPLATE/CHERRY_PICK.md file on this repo.

Then we can share the same content while keeping a direct template to start with instead of manually copy-paste.

See more information here.

Saying that I do not have the permission to create the repo.

Do we need to re-disccuss this again on the next Contributor Experience Workgroup meeting? @AnthonyLatsis

@AnthonyLatsis
Copy link
Contributor Author

I'd like to suggest we add a new .github repo on swiftlang and add PULL_REQUEST_TEMPLATE/CHERRY_PICK.md file on this repo.

Yep, that was the original idea — suboptimal for apple, but much better for swiftlang. We’re going to discuss this next meeting (I was the one to originally bring up this topic and nobody seemed to be anticipating swiftlang at the time).

@parispittman Do we have a rough estimate on the time frame of repository transfers to swiftlang?

@Kyle-Ye
Copy link
Contributor

Kyle-Ye commented Jun 18, 2024

I'd like to suggest we add a new .github repo on swiftlang and add PULL_REQUEST_TEMPLATE/CHERRY_PICK.md file on this repo.

Yep, that was the original idea — suboptimal for apple, but much better for swiftlang. We’re going to discuss this next meeting (I was the one to originally bring up this topic and nobody seemed to be anticipating swiftlang at the time).

@parispittman Do we have a rough estimate on the time frame of repository transfers to swiftlang?

I saw the transition has already began.

I suggest we just use "the original idea".

For those repo still hosted on apple currently (eg. swift-docc), we can continue use the old pattern.

When the migration is done for them, we can then just safely delete the duplicated .github/PULL_REQUEST_TEMPLATE/CHERRY_PICK.md file on those repo.

@AnthonyLatsis
Copy link
Contributor Author

Turns out, folks have already added a .github repository to swiftlang, so I went ahead and added the org-wide pull request template. It works as a query parameter just fine. The template is called release.md — I think this neutral and simple if straightforward name will be easiest for us all to remember and type. I will update this pull request accordingly as soon as the repo moves to swiftlang.

Thanks for weighing in!

@AnthonyLatsis AnthonyLatsis changed the title [NFC] .github: Link out to Swift.org for release branch PR form [NFC] Delete release branch PR template in favor of org-wide variant Jul 12, 2024
Copy link
Contributor

@Kyle-Ye Kyle-Ye left a comment

Choose a reason for hiding this comment

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

Another thing I'd like to confirm is that after your PR we should continue to be able to create cherry-pick/release template PR via GitHub's official command line tool - gh.

image

But it seems we will lose the template discovery after your PR. (Delete CHERRY_PICK.md but not adding swiftlang/.github's release.md)

I do not know if this a bug on gh or a misconfiguration on swiftlang/.github repo.

See discussion here for more information #300 (comment) #300 (comment)

`swiftlang` now has an equivalent organization-wide PR template, so we
no longer need this local one. Also, prompt folks to use this template
when targeting a release branch in the default PR template.
@AnthonyLatsis
Copy link
Contributor Author

AnthonyLatsis commented Jul 17, 2024

Confirmed with the swift repository, which no longer has a release template.

Screenshot 2024-07-17 at 02 53 46

But it seems we will lose the template discovery after your PR.

Because this repository will no longer have its own template, you mean? I believe that is fine:

  • We still have a leading comment in the default template.
  • Folks that submit changes to release branches on a regular basis have the experience to catch on.
  • First-timers are in the majority of cases asked by code owners to cherry-pick something and receive appropriate guidance. One can also learn about the template and how to tap into it from the website: https://www.swift.org/contributing/#release-branch-pull-requests.

@AnthonyLatsis
Copy link
Contributor Author

AnthonyLatsis commented Jul 17, 2024

Confirmed with the swift repository, which no longer has a release template.

It seems I was wrong. I tried again with the Apple main branch (as I should have initially) as the base and now it only shows the default template. release.md is only shown when the repository has no PR templates at all, like swift-syntax. To make things worse, --template release.md --web does not add the template query parameter to the URL even if the template exists. 😕

cli/cli#9325

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