Skip to content

Add note that LLDB changes should be cherry-picked to swift/master #32133

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

Merged
merged 5 commits into from
Jun 4, 2020

Conversation

martinboehme
Copy link
Contributor

This was previously missing -- the doc mentioned only swift/master-next and release branches.

Unverified

This user has not yet uploaded their public signing key.
@martinboehme martinboehme requested a review from hyp June 2, 2020 08:07
@martinboehme
Copy link
Contributor Author

@swift-ci Please smoke test

@gribozavr gribozavr requested a review from fredriss June 2, 2020 08:10
@gribozavr
Copy link
Contributor

LGTM, but I would like us to get a review from someone working on Swift-enabled LLDB like @fredriss.

@martinboehme
Copy link
Contributor Author

@swift-ci Please smoke test Linux

@fredriss
Copy link
Contributor

fredriss commented Jun 2, 2020

I don't think this reflects how anyone is working today. We work mainly on swift/master and move to swift/release/x.y when it branches. For sake of the discussion, here are the current relevant automergers:

  • llvm.org:master -> github/apple:apple/master -> github/apple:swift/master-next
  • github/apple:apple/stable/20200801 -> swift/release/5.3 -> swift/master

The forwarding to swift/master will disappear later in the release cycle.

The rule of thumb we use for LLDB is:

  • If something is generic enough, do it upstream on llvm.org. It will end-up in swift/master-next automatically. Then cherry-pick to the appropriate branch, depending on which release it's targeted at. For changes to go into 5.3, this branch is apple/stable/20200801.
  • If it cannot do it upstream, do it on either the release branch or swift/master and cherry-pick it to swift/master-next.

It would be nice to be able to live on master-next, but we're not there yet.

@martinboehme
Copy link
Contributor Author

I don't think this reflects how anyone is working today.
[...]
If it cannot do it upstream, do it on either the release branch or swift/master and cherry-pick it to swift/master-next.

Thanks for the input!

Just to confirm: This means that the current doc (without my PR) is also already wrong, as it says

"Changes that depend on Swift (this only applies to LLDB): new commits go to swift/master-next".

You're saying this should instead read

"New commits go to swift/master"

Correct?

@fredriss
Copy link
Contributor

fredriss commented Jun 2, 2020

You're saying this should instead read

"New commits go to swift/master"

Correct?

In essence, yes. But it's a little more nuanced than this. There's nothing really wrong with committing first to swift/master-next. The issue is that the branch is not fully livable yet, so people don't work on it.

The really important thing is that commits should go to both swift/master (either directly or through the current release branch), and swift/master-next. The order doesn't really matter. But given the current state of things I think it's fine to document that commits should go to swift/master first.

Unverified

This user has not yet uploaded their public signing key.
…aster-next`.
@martinboehme
Copy link
Contributor Author

In essence, yes. But it's a little more nuanced than this. There's nothing really wrong with committing first to swift/master-next. The issue is that the branch is not fully livable yet, so people don't work on it.

The really important thing is that commits should go to both swift/master (either directly or through the current release branch), and swift/master-next. The order doesn't really matter. But given the current state of things I think it's fine to document that commits should go to swift/master first.

Thanks, got it!

I've changed the wording in the doc accordingly -- WDYT?

All of this is very relevant to me right now, BTW, since I'm preparing a (Swift-dependent) change for LLDB (not surprisingly, this is what triggered me to make this change to the doc). I've currently got it on swift/master-next, so I may well go ahead with committing from there first.

@martinboehme
Copy link
Contributor Author

@swift-ci Please smoke test

@fredriss
Copy link
Contributor

fredriss commented Jun 3, 2020

This looks good with one last tweak (sorry!). The current release branches are not named swift/swift-x.y-branch anymore, but swift/release/x.y.

Unverified

This user has not yet uploaded their public signing key.
@martinboehme
Copy link
Contributor Author

This looks good with one last tweak (sorry!). The current release branches are not named swift/swift-x.y-branch anymore, but swift/release/x.y.

No problem. How does this look now?

Unverified

This user has not yet uploaded their public signing key.
@martinboehme
Copy link
Contributor Author

@swift-ci Please smoke test

@martinboehme
Copy link
Contributor Author

@swift-ci Please smoke test

@martinboehme martinboehme merged commit 4e0fad8 into master Jun 4, 2020
@martinboehme martinboehme deleted the martinboehme-patch-1 branch June 4, 2020 16:31
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