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

scripts: fix the release note script to pick up more backports #43302

Merged
merged 1 commit into from
Dec 18, 2019

Conversation

knz
Copy link
Contributor

@knz knz commented Dec 18, 2019

Prior to this patch, the release note script was only recognizing PR
merges from either of two formats:

  • from Bors, starting with Merge #xxx #yyy
  • from Github, starting with Merge pull request #xxx from ...

Sometime in 2019, Github has started populating merge commits using
a different format, using instead the title of the PR followed by the
PR number between parentheses.

This new format was not recognized by the script and caused it to skip
over these merges (and all their underlying commits) and list them in
the section "changes without a release note annotation".

This patch fixes it.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

Prior to this patch, the release note script was only recognizing PR
merges from either of two formats:

- from Bors, starting with `Merge #xxx #yyy`
- from Github, starting with `Merge pull request #xxx from ...`

Sometime in 2019, Github has started populating merge commits using
a different format, using instead the title of the PR followed by the
PR number between parentheses.

This new format was not recognized by the script and caused it to skip
over these merges (and all their underlying commits) and list them in
the section "changes without a release note annotation".

This patch fixes it.

Release note: None
Copy link
Contributor

@jseldess jseldess left a comment

Choose a reason for hiding this comment

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

:lgtm: but I didn't test it. Have you run it a few times, @knz?

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @ericharmeling and @jseldess)

@knz
Copy link
Contributor Author

knz commented Dec 18, 2019

Yes I have verified it picks up the missing bits on 2.1.10 and 19.1.6.

@knz
Copy link
Contributor Author

knz commented Dec 18, 2019

bors r+

craig bot pushed a commit that referenced this pull request Dec 18, 2019
43032: kv: remove `GetMeta`, `AugmentMeta` and `TxnCoordMeta` r=knz a=knz

Recommended/requested by @nvanbenschoten. 
Discussed with @andreimatei. 
Prerequisite to completing #42854.

Prior to this patch, the same data structure `TxnCoordMeta` was used
both to initialize a LeafTxn from a RootTxn, and a RootTxn from a
LeafTxn. Moreover, the same method on `TxnCoordSender` (`AugmentMeta`)
was used to "configure" a txn into a root or a leaf, and to update
a root from the final state of leaves.

This was causing difficult questions when adding features (all the
fields in TxnCoordMeta needed to produce effects in one direction
and no-ops in the other). It was also making it hard to read and
understand the API.

This patch alleviates this problem by separating the two protocols:

```go
// From roots:
func (txn *Txn) GetLeafTxnInputStateOrRejectClient(context.Context) (roachpb.LeafTxnInputState, error)

// to create a new leaf:
func NewLeafTxn(context.Context, *DB, roachpb.NodeID, *roachpb.LeafTxnInputState) *Txn

// From leaves, at end of use:
func (txn *Txn) GetLeafTxnFinalState(context.Context) roachpb.LeafTxnFinalState

// Back into roots:
func (txn *Txn) UpdateRootWithLeafFinalState(context.Context, tfs *roachpb.LeafTxnFinalState)
```

Additionally, this patch:

- removes the general-purpose `Serialize()` method, and replaces it
  by `TestingCloneTxn()` specifically purposed for use in testing.

- removes direct access to the TxnMeta `WriteTimestamp` in the SQL
  conn executor (to establish a high water mark for table lease
  expiration), and replaces it by a call to a new method
  `ProvisionalCommitTimestamp()`.

Release note: None

43296: sql: support EXPLAIN with AS OF SYSTEM TIME r=RaduBerinde a=RaduBerinde

We apparently can't stick an `EXPLAIN` in front of a query that uses
AOST. The fix is very easy, we need an extra case for the logic that
figures out the statement-wide timestamp.

Note that if we want to do `SELECT FROM [EXPLAIN ...]`, in that case
we still need to add AS OF SYSTEM TIME to the outer clause as usual.

Fixes #43294.

Release note (bug fix): EXPLAIN can now be used with statements that
use AS OF SYSTEM TIME.

43300: blobs: Stat method bug fix r=g3orgia a=g3orgia

The stat method has a typo/bug, it return nil
instead of err. This PR fixes it.

Release note: None

43302: scripts: fix the release note script to pick up more backports r=knz a=knz

Prior to this patch, the release note script was only recognizing PR
merges from either of two formats:

- from Bors, starting with `Merge #xxx #yyy`
- from Github, starting with `Merge pull request #xxx from ...`

Sometime in 2019, Github has started populating merge commits using
a different format, using instead the title of the PR followed by the
PR number between parentheses.

This new format was not recognized by the script and caused it to skip
over these merges (and all their underlying commits) and list them in
the section "changes without a release note annotation".

This patch fixes it.

Release note: None

Co-authored-by: Raphael 'kena' Poss <[email protected]>
Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: Georgia Hong <[email protected]>
@craig
Copy link
Contributor

craig bot commented Dec 18, 2019

Build succeeded

@craig craig bot merged commit 505be09 into cockroachdb:master Dec 18, 2019
@ericharmeling
Copy link
Contributor

@knz Thank you!

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.

4 participants