Create a migration schedule for GitHub Pull Requests #63295
Comments
|
will llvm accept github pull requests soon? |
EugeneZelenko
added
infrastructure
|
@llvm/issue-subscribers-infrastructure |
|
I discussed this with @tlattner @asl @rnk @llvm-beanz @kbeyls on a video call. We discussed this approximate schedule:
|
|
@tstellar thanks. So excited! |
|
Why do we keep Phabricator between October 1 2023 and 2024? Since it is "Read-only" that means that the intent isn't to be able to continue commenting on already committed revision right? As soon as it is read-only we should be able to start mirroring it to static HTML pages that we should publish as a static website that is kept around "forever", preserving access through existing URL. Then we can just tear-down the server at this point... Another question on the timeline is: what is the status on all the open questions that there was on PR workflows? |
|
We need full documentation on the accepted procedure for creating PRs added to https://www.llvm.org/docs/Contributing.html - this needs to be done in plenty of time before the Sept 1 deadline, mid July at the latest probably as I can imagine a lot of people will need hand holding. Do you have to create PRs from your own forks or are we allowing any branch to be pushed upstream now? As I understood it anything but the official branches were likely to be culled at anytime? |
|
@RKSimon There is a guide currently: https://llvm.org/docs/GitHub.html?highlight=github All docs will need to be reviewed and updated to point to that document. |
|
@joker-eph Ideally we do not want to keep it around or keep the maintenance burden there. Discussions with Phab maintainers (you and others) need to be had to understand what is possible and what is not. Anton also mentioned we could export to static html, so if thats easy to do then that would for sure would be a great path to take. @tstellar Should we make another issue to track this? |
Would it be better to remove that GitHub page entirely and merge the contents into Contributing? |
This is the preferred outcome, but we weren't sure if this is something that Phabricator supports. |
I think the two outstanding issues are: When is it OK to have multiple commits in a pull request and how to support stacked reviews. |
I think the point of keeping it around in readonly mode for a year is exactly to allow time for doing exactly that: developing a static HTML export script which doesn't exist right now, so far as I know. If someone writes a good export tool, maybe we can do it sooner.
Open questions such as all the many points in llvm/llvm-iwg#73 ? I think they are still open. I think Phabricator is hard to compete with, it's a good tool! It's just not the standard flow that new contributors expect anymore. |
|
Maybe I'm missing something, but isn't a migration schedule a bit premature until we have a firm solution to the stacked PR situation? What happens if we get to September and we haven't come to a consensus on it? I am frequently reviewing Phabricator patches that are in a stack, so this isn't just an issue that can be ignored. |
|
@jh7370 Take a look at the initial announcment. The plan is to move forward even if some of these features are still missing. #56636 is the place to discuss how to implement stacked code reviews given GitHub's current feature set. |
|
@tstellar what is the tracker for what is / isn't a blocker right now? There are things that look hard blocker to me like #56637 ; I'm worried about what will go in the repo if we open the possibility to merge PR without locking them down to "squash&rebase only" and "use PR description for commit message". |
|
I have definitely found on a large team that it's easy for folks to forget when to switch the merge button between "merge" and "squash & merge". It seems reasonable to only allow squash and merges, and if folks want multiple commits they can create multiple PRs (given no better solution from github itself). If that workflow didn't work out it could always become more permissive after the fact |
|
@joker-eph The plan was no blockers, so if there are major problems let's try to get them fixed. If we enable "squash@rebase only" that means you can create a pull request with multiple commits, but would only be allowed to push a single squashed commit, is that correct? |
|
Yes squash and merge with commit message being PR title and description is IMO the closest to the current phab workflow. Allowing rebase and merge retains all the commits from the PR on main. |
|
Was this announced on the forums? Searching for "pull requests" only brings me @joker-eph's original proposal when we were still using mailing lists: https://discourse.llvm.org/t/enable-contributions-through-pull-request-for-llvm/53627 I really want to avoid the same problem we had with the mailing list move, and September isn't really that far away. I'd hate if people only figured it out in August... |
|
Here is the discourse thread I believe: https://discourse.llvm.org/t/update-on-github-pull-requests/71540 |
|
Ah, I did not see that. Would be good to keep that more high-profile, in the Announcements category or something. |
|
I have posted this on Announcements. https://discourse.llvm.org/t/pull-request-migration-schedule/71595 Keep in mind this is a discussion about the schedule though and we are requesting community feedback. Once finalized, there will be very frequent and high profile reminders. |
|
Closing as the transition has been made: https://discourse.llvm.org/t/pull-requests-enabled-for-llvm-llvm-project-repo/73186 |
We need to create a migration schedule for moving to GitHub Pull Requests. It should include things like when to open up pull requests for experimentation, when to shut off Phabricator, etc. I think it may be best to discuss this on Discourse so it gets the most visibility.
The text was updated successfully, but these errors were encountered: