docs(MAINTAINERS): Cover pre-release generation

[gibson042]

closes: #9234

Description

Update MAINTAINERS.md and scripts/get-released-tags to be more descriptive about expected use.

Security Considerations

n/a

Scaling Considerations

n/a

Documentation Considerations

Included in code changes.

Testing Considerations

None.

Upgrade Considerations

Non-technical processes such as internal handoff are not included and are expected to remain in upgrade runbooks.

[cloudflare-workers-and-pages]

Deploying agoric-sdk with    Cloudflare Pages

Latest commit:	676098a
Status:	✅  Deploy successful!
Preview URL:	https://2d8a2fd9.agoric-sdk.pages.dev
Branch Preview URL:	https://gibson-9234-improve-release.agoric-sdk.pages.dev

View logs

[mhofman]

The shell changes look fine, but I haven't tried to replicate

[gibson042]

@mhofman Please pay particular attention to the changes in this section.

[gibson042]

@mhofman I'm re-requesting review, because there have been substantial changes per our earlier discussion.

[mhofman]

Thanks for improving these docs. I need to do another pass, and figure out what else I had missing in the draft.

[mhofman]

I know I wrote this originally, but I think the "no merge back" only applies for this long running release fork. I do expect that release branches in the future will be short lived, and merged back to master.

[gibson042]

I want this to document current practices rather than hopeful improvements. A great deal of changes will be required when we switch to that model.

[mhofman]

Nit picking here, but the upgrade handler/plan name does not need to be exactly the "release label", and it actually isn't always the same since the plan name for the "test" network upgrades is not the same as rc git tags.

Maybe we should change one to make it clear git tags != plan name ?

[mhofman]

I think here we need to also remove any logic specific to the previous upgrade (e.g. core proposals)

[mhofman]

I think an example might help here use-upgrade-7 (if we keep using upgrade-8 as example)

[mhofman]

planName definitely needs to be the "upgrade plan name". The conflation with the git tag right now has been bother me.

[mhofman]

I still owe the regex to build the todo log

[mhofman]

Here is how I've been generating the todo log:

• Switch to master branch
• git rebase -i --rebase-merges $LAST_COMMIT_IN_PREVIOUS_LOG
• Run the following substitution to "organize" the todo log
  • \n# Branch (.*)\nreset .*\n(merge -C .*\n)?((?:pick .*\(#\d+\)\n)*)(?:label branch-point.*\n)?((?:pick .*\n)*)(?:merge -C .*\n)?(?:label \1\n) -> $2\n$3\n# Branch $1\nlabel base-$1\n$4label $1\nreset base-$1\n
  • Manually fix the last block
  • Add comments for squashed PRs
  • Copy the resulting todo
• Run the rebase and verify the commit graph does not change (aka that the re-organization didn't change anything)

[mhofman]

Was this useful for dev-upgrade-15?

[gibson042]

Hmm, I did not recognize your todo lists as derived from something produced by git and instead just manually based #9264 and #9266 on what you had done with upgrade-14 PRs like #8942. But even looking now, I don't think I understand where $LAST_COMMIT_IN_PREVIOUS_LOG is coming from, or how those "reorganizations" would be applied. Is the latter scriptable in a way that could be explained and checked in?

[gibson042]

Is the latter scriptable in a way that could be explained and checked in?

A possibility: restructure blocks, then rename labels.

awk '
  # Restructure branch-specific blocks from `git rebase -i --rebase-merges`
  # so that `merge` commands appear at block end rather than block start.

  NR == 1 && match($0, /^label /) {
    # An initial `label` (but not a following no-op `reset`) belongs in the first block.
    firstBlockPrefix = $0 "\n";
    noopReset = "reset " substr($0, RLENGTH + 1, length($0) - RLENGTH);
    next;
  }
  firstBlockPrefix != "" && $0 == noopReset {
    next;
  }
  /^$|^# Branch .*/ {
    # TODO: We may be able to find the PR number from a commit id or branch name.
    sub(/Branch/, "PR #??? Branch");
    blockHeader = blockHeader $0 "\n";
    next;
  }
  !match($0, /^(reset|merge -C) /) {
    printf "%s%s", blockHeader, firstBlockPrefix;
    blockHeader = "";
    firstBlockPrefix = "";
  }
  {
    print;
  }
  END {
    printf "%s%s", blockHeader, firstBlockPrefix;
  }
' | awk '
  # Rename each label that receives a `merge` to "base-$branchName".

  function addLabel(label) {
    if (++labels[label] == 1) return;
    print "duplicate label: " label > "/dev/stderr";
    exit 1;
  }
  match($0, /^$|^# .*[Bb]ranch /) {
    branch = substr($0, RLENGTH + 1, length($0) - RLENGTH);
  }
  branch != "" && match($0, /^merge -C /) && match(prev, /^reset /) {
    onto = substr(prev, RLENGTH + 1, length($0) - RLENGTH);
    sub(/ .*/, "", onto);
    newOnto = "base-" branch;
    addLabel(newOnto);
    renames[++renameCount] = onto SUBSEP newOnto;
  }
  /^label / {
    addLabel(substr($0, RLENGTH + 1, length($0) - RLENGTH));
  }
  {
    buf = buf $0 "\n";
    prev = $0;
  }
  END {
    # For "last-wins" semantics, apply renames in reverse order.
    for (i = renameCount; split(renames[i], pair, SUBSEP); i--) {
      for (j = split("label reset", rebaseCmds, " "); cmd = rebaseCmds[j]; j--) {
        newBuf = "";
        seekLine = sprintf("\n%s %s", cmd, pair[1]);
        while (k = index(buf, seekLine)) {
          len = length(seekLine);
          r = sprintf("\n%s %s", cmd, pair[2]);
          if (!match(substr(buf, k + len, 1), /[ \n]/)) r = seekLine;
          newBuf = newBuf substr(buf, 1, k - 1) r;
          buf = substr(buf, k + len, length(buf) - (k - 1) - len);
        }
        buf = newBuf buf;
      }
    }
    printf "%s", buf;
  }
'

[mhofman]

This seems partially duplicated.

It's not just test files, but really the whole folder, including package.json.

[mhofman]

I think we can remove this.

[mhofman]

It's unclear what to do with this edit, especially given the subsequent request to delete the checkout.

I think this section is better placed as a sub-bullet of copying over the test proposal to agoric-3-proposal, as one of the modifications to be made.

Other modifications:

• set the releaseNotes to the expected URL
• generate the expected upgradeInfo from the script

[gibson042]

Done.

[mhofman]

Was this useful for dev-upgrade-15?

[mhofman]

I think we no longer have this.

[mhofman]

In my release PRs I have been trying to include hidden in a details tag, the formatted rebase todo of master since the last release was cut, so we can more easily see which PRs were included, and which weren't, and in the future, just reformat since that point only to avoid duplicate work.

[gibson042]

Can you elaborate?

[mhofman]

We should probably have the PR labelled force:integration (at least until we have better CI logic to detect these kind of release PRs).

The idea is that integration tests should pass before we publish, which happens before we attempt to merge.

[mhofman]

Commenting here but it's about the section above. I believe publish to NPM should only happen after the publish PR is approved. I thought this was already in the Maintainers file, but it might only be in the run book.