[tools] Avoid new_release upgrades newer than one week

[tyler-yankee]

Along the lines of #24558, applied to the new_release tool when fetching from GitHub and other sources.

Towards #24555.

Requires #24593, #24645.

---

This change is 

[tyler-yankee]

+a:@mwoehlke-kitware for feature review, please.

@tyler-yankee reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

[tyler-yankee]

@tyler-yankee made 1 comment.
Reviewable status: LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

---

a discussion (no related file):
FYI This only applies the "too-new" logic to tags and releases, not pinned commits off of master or whatever. I wasn't sure whether we wanted to pursue the latter? @jwnimmer-tri

[mwoehlke-kitware]

@mwoehlke-kitware made 6 comments.
Reviewable status: 6 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

---

tools/workspace/new_release.py line 89 at r1 (raw file):

# For these repositories, ignore any tags that match the specified regex.
_IGNORED_TAGS = {
    "dm_control_internal": r"^[^0-9].*$",

re.match already only matches at start. I also don't see much benefit to forcing the regex to match the entire string.

Suggestion:

r"[^0-9]"

---

tools/workspace/new_release.py line 199 at r1 (raw file):

    development_stages = ["alpha", "beta", "pre", "rc", "unstable"]
    prerelease = any(stage in commit for stage in development_stages)
    if prerelease:

...since prerelease is no longer used twice...

Suggestion:

    if any(stage in commit for stage in development_stages):

---

tools/workspace/new_release.py line 208 at r1 (raw file):

        # Avoid hot-off-the-press releases as potential for malware, but log it
        # for the user to check.
        warn(f"Skipping fresh release {commit} for {workspace}")

Suggestion:

too-recent

---

tools/workspace/new_release.py line 219 at r1 (raw file):

        commit = tag.name
        commit_date = _get_commit_date(gh_repo, commit)
        if _is_ignored_tag(commit, commit_date, workspace):

Consider?

Suggestion:

        tag_date = _get_commit_date(gh_repo, tag.name)
        if _is_ignored_tag(tag.name, tag_date, workspace):

---

tools/workspace/new_release.py line 274 at r1 (raw file):

        # we're trying to capture.
        if _is_ignored_tag(new_commit, release.published_at, workspace_name):
            new_commit = _latest_tag(gh_repo, workspace_name)

BTW, this fallback logic seems odd. Shouldn't we check older releases before resorting to only considering tags? It seems like the original thought here might have been that we would only fall back to tags if there were no releases, but now that we're being more picky about when we'll accept a release, I'm worried we may be introducing an unintended fallback to tags.

---

tools/workspace/new_release.py line 258 at r1 (raw file):

                        _get_commit_date(gh_repo, tag.name),
                        workspace_name,
                    ):

Consider?

Suggestion:

                    tag_date = _get_commit_date(gh_repo, tag.name)
                    if _is_ignored_tag(tag.name, tag_date, workspace_name):

[tyler-yankee]

@tyler-yankee reviewed 1 file and all commit messages, made 1 comment, and resolved 5 discussions.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

---

tools/workspace/new_release.py line 274 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

BTW, this fallback logic seems odd. Shouldn't we check older releases before resorting to only considering tags? It seems like the original thought here might have been that we would only fall back to tags if there were no releases, but now that we're being more picky about when we'll accept a release, I'm worried we may be introducing an unintended fallback to tags.

Yes, agreed. I can explore this

[tyler-yankee]

@tyler-yankee reviewed 1 file and all commit messages, and made 1 comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

---

tools/workspace/new_release.py line 274 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Yes, agreed. I can explore this

I've updated it to iterate over the 3 most recent releases before falling back to tags. PTAL, thanks!

[mwoehlke-kitware]

@mwoehlke-kitware made 1 comment and resolved 1 discussion.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

---

tools/workspace/new_release.py line 259 at r3 (raw file):

    # Check the latest few releases (in case the latest meets one of our
    # exclusion criteria), and fallback to tags if releases are unavailable.

Minor:

Suggestion:

fall back

[tyler-yankee]

@tyler-yankee reviewed 1 file and all commit messages, and resolved 1 discussion.
Reviewable status: LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

[mwoehlke-kitware]

@mwoehlke-kitware made 1 comment.
Reviewable status: needs platform reviewer assigned, needs at least two assigned reviewers (waiting on tyler-yankee).

[tyler-yankee]

+a:@jwnimmer-tri do you want to take platform here, or re-assign to on-call?

@tyler-yankee made 1 comment.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform) (waiting on jwnimmer-tri).

[jwnimmer-tri]

I can review.

@jwnimmer-tri reviewed all commit messages and made 3 comments.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform) (waiting on tyler-yankee).

---

a discussion (no related file):

Previously, tyler-yankee (Tyler Yankee) wrote…

FYI This only applies the "too-new" logic to tags and releases, not pinned commits off of master or whatever. I wasn't sure whether we wanted to pursue the latter? @jwnimmer-tri

Yes, we should also avoid bumping to the head of master until master has been quiescent for at least 7 days.

---

a discussion (no related file):
Here's a question / idea -- I haven't tried to pencil it out, but I wonder if it ends up being simpler...

Instead of using heuristics for whether a repository uses full GitHub releases or just GitHub tags, we could have a lookup table in new_release that selects which to use (or fails-fast if not specified, i.e., when adding a new dependency).

That might remove the need for IGNORED_TAGS and OVERLOOK_RELEASE_REPOSITORIES while also simplifying the "newer than date" logic to not need to worry about both at the same time.

Would that simplify things?

[tyler-yankee]

@tyler-yankee made 1 comment.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform) (waiting on mwoehlke-kitware and tyler-yankee).

---

a discussion (no related file):
Having a hard-coded lookup table would be sort of along a similar vein/philosophy as #23530. It might be less flexible if an upstream decides to change their release patterns though? (I'm not sure how common that case would be, but right now new_release would just handle that dynamically.)

simplifying the "newer than date" logic to not need to worry about both at the same time.

Correct.

might remove the need for _IGNORED_TAGS and _OVERLOOK_RELEASE_REPOSITORIES

Hm, I'm not sure I follow. They're essentially doing tag filtering, which would need to be kept in some form (especially the "pin to a given major or major.minor release series" bits)?

I can start trying to sketch it out locally and see if it simplifies any of our heuristics, but fwiw it seems to me like it would be an improvement mostly independent of this PR, that happens to simplify the ~50 lines being added here as a byproduct. @mwoehlke-kitware any thoughts to chime in?

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform) (waiting on mwoehlke-kitware and tyler-yankee).

---

a discussion (no related file):

It might be less flexible if an upstream decides to change their release patterns though? (I'm not sure how common that case would be, but right now new_release would just handle that dynamically.)

Yes, I think it would be uncommon. For "handle that dynamically" is not clear from the code (especially with no tests) that it would work; depending on how upstream evolves things it's difficult to reason about whether we would correctly adapt automatically.

might remove the need for _IGNORED_TAGS and _OVERLOOK_RELEASE_REPOSITORIES

Hm, I'm not sure I follow.

Right. I guess some of the regexs would need to stay (for filtering purposes); perhaps _IGNORED_TAGS is unchanged entirely. But the associated heuristic of "For these repositories, we only look at tags, not releases" for _OVERLOOK_RELEASE_REPOSITORIES would go away.

... that happens to simplify the ~50 lines being added here as a byproduct ...

The bottom line is that reasoning about the behavior of the current code requires a fairly complicated mental model with lots of moving pieces, but this tool is supposed to be simple and boring. The way the new logic got added seems like it tipped the scale towards "unmaintainable", and it's time to simplify and streamline.

[tyler-yankee]

@tyler-yankee made 1 comment.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), labeled "do not merge" (waiting on mwoehlke-kitware).

---

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

It might be less flexible if an upstream decides to change their release patterns though? (I'm not sure how common that case would be, but right now new_release would just handle that dynamically.)

Yes, I think it would be uncommon. For "handle that dynamically" is not clear from the code (especially with no tests) that it would work; depending on how upstream evolves things it's difficult to reason about whether we would correctly adapt automatically.

might remove the need for _IGNORED_TAGS and _OVERLOOK_RELEASE_REPOSITORIES

Hm, I'm not sure I follow.

Right. I guess some of the regexs would need to stay (for filtering purposes); perhaps _IGNORED_TAGS is unchanged entirely. But the associated heuristic of "For these repositories, we only look at tags, not releases" for _OVERLOOK_RELEASE_REPOSITORIES would go away.

... that happens to simplify the ~50 lines being added here as a byproduct ...

The bottom line is that reasoning about the behavior of the current code requires a fairly complicated mental model with lots of moving pieces, but this tool is supposed to be simple and boring. The way the new logic got added seems like it tipped the scale towards "unmaintainable", and it's time to simplify and streamline.

That's a fair assertion. I haven't had time to look into this today but I'll try to sketch a separate branch next week.

+(status: do not merge) +(status: do not review)

[mwoehlke-kitware]

we should also avoid bumping to the head of master until master has been quiescent for at least 7 days

I don't know about our dependencies (that don't have release tags) specifically, but for some projects, the chances of that happening on a regular basis, even if we were checking for updates daily (which we aren't), is virtually nil. If we're running this script once a month, the chances that we'll "just happen" to catch a quiescent period more often than not seem small, which would likely result in us never finding a new 'stable' update, or at least, not finding one often enough to not defeat the purpose of the update script.

Taking 'master as of a week ago' doesn't work either, since there's no guarantee any security issues get expunged from the history.

Maybe we could look for the first commit that wasn't followed-up within a week? But do all our dependencies (at least, that would use this logic) have commits sufficiently infrequent for this to work?

Consider VTK for example; I would be quite surprised if the gaps with at least a week between commits are not extremely spread out, if indeed they exist at all.

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), labeled "do not merge" (waiting on mwoehlke-kitware and tyler-yankee).

---

a discussion (no related file):

Consider VTK for example ..

Good point, I didn't think about VTK. For that case, we'd need a "trust me" list with VTK on it, where the 7-day-quiescent requirement is ignored (and we'd just use head of master).

For the rest of our dependencies, they are low bandwidth and I'd be comfortable leaving the dwell time enforced.

[tyler-yankee]

@tyler-yankee reviewed 66 files and all commit messages, and made 2 comments.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on mwoehlke-kitware and tyler-yankee).

---

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Consider VTK for example ..

Good point, I didn't think about VTK. For that case, we'd need a "trust me" list with VTK on it, where the 7-day-quiescent requirement is ignored (and we'd just use head of master).

For the rest of our dependencies, they are low bandwidth and I'd be comfortable leaving the dwell time enforced.

Still on me to create the "trust me" list as described above and otherwise apply the too-recent logic to commits from the mainline branch.

---

a discussion (no related file):

Previously, tyler-yankee (Tyler Yankee) wrote…

That's a fair assertion. I haven't had time to look into this today but I'll try to sketch a separate branch next week.

+(status: do not merge) +(status: do not review)

I've pushed the changes for the whole PR train to this branch, now just waiting on separate PRs to land and I'll rebase on master.

[tyler-yankee]

The preceding commit is cherry-picked from #24613 to allow for local testing, but it's not strictly dependent on it, if this is ready to land first we can do so independently.

As of writing, the output for "too recent" is:

Skipping too-recent f50b1fd1e7b940f0c4e0c91d7e39fc0f15c45881 for common_robotics_utilities_internal
Skipping too-recent 1.0.43 for dm_control_internal
Skipping too-recent 1.0.42 for dm_control_internal
Skipping too-recent 4723170e1c8ee8bc59c342bb5c491ab8f263f1fb for drake_models
Skipping too-recent 45636bdcef1a4fec140346b90c0b50bf0bc3e23b for tinyobjloader_internal
Skipping too-recent 477cbdfb9bc220c56d1453c4a7b00686ebbce88b for tinyobjloader_internal
Skipping too-recent 68aa0d6194c212463b547ac021a72fbd5d0c4b8c for tinyobjloader_internal

Back to @mwoehlke-kitware and @jwnimmer-tri for a re-review, please. Thank you!

@tyler-yankee reviewed 67 files and all commit messages, made 2 comments, and resolved 1 discussion.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), labeled "do not merge", commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on jwnimmer-tri).

---

a discussion (no related file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Still on me to create the "trust me" list as described above and otherwise apply the too-recent logic to commits from the mainline branch.

Done.

[jwnimmer-tri]

@jwnimmer-tri reviewed 66 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on tyler-yankee).

---

tools/workspace/new_release.py line 72 at r7 (raw file):

# because we depend on arbitrary commits from the mainline branch, and it's not
# practical to assume no activity in that period of time.
_IGNORE_RECENT_RELEASE = [

In the spirit of pushing repository-specific details out to the repositories themselves, what if we expanded the UpgradeType to have commit and commit_without_cooldown (or with whatever names you like)? That seems more aligned with the current design.

[tyler-yankee]

@tyler-yankee made 1 comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on jwnimmer-tri).

---

tools/workspace/new_release.py line 72 at r7 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

In the spirit of pushing repository-specific details out to the repositories themselves, what if we expanded the UpgradeType to have commit and commit_without_cooldown (or with whatever names you like)? That seems more aligned with the current design.

A couple thoughts here:

• Is there a future in which such "skip cooldown" logic also applies to tags and releases, in which case it would be better to have a separate flag instead of duplicating UpgradeTypes?
• Bigger picture, which of these top-level fields make sense to be eventually moved to repo metadata? I could see the case for all of them, possibly except _COHORTS (does a single package list what other packages are in its cohort, in which case they should all match each other, but it's unclear to me how to enforce that).

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on tyler-yankee).

---

tools/workspace/new_release.py line 72 at r7 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

A couple thoughts here:

• Is there a future in which such "skip cooldown" logic also applies to tags and releases, in which case it would be better to have a separate flag instead of duplicating UpgradeTypes?
• Bigger picture, which of these top-level fields make sense to be eventually moved to repo metadata? I could see the case for all of them, possibly except _COHORTS (does a single package list what other packages are in its cohort, in which case they should all match each other, but it's unclear to me how to enforce that).

I would be OK with an approach where upgrade_cooldown_days was optional additional data that could be dumped into the json when present. (It's repository rule attr should not set mandatory = True.)

[tyler-yankee]

@tyler-yankee reviewed 6 files and all commit messages, and made 1 comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform) (waiting on jwnimmer-tri).

---

tools/workspace/new_release.py line 72 at r7 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I would be OK with an approach where upgrade_cooldown_days was optional additional data that could be dumped into the json when present. (It's repository rule attr should not set mandatory = True.)

OK, I've implemented this. thanks!

[tyler-yankee]

@tyler-yankee made 1 comment.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform) (waiting on jwnimmer-tri and mwoehlke-kitware).

---

tools/workspace/new_release.py line 245 at r8 (raw file):

            commit_date = _get_commit_date(gh_repo, sha)
            if not _is_ignored_tag(
                commit.sha,

nit

Suggestion:

                sha,

[jwnimmer-tri]

@jwnimmer-tri reviewed 2 files and all commit messages, made 3 comments, and resolved 1 discussion.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform) (waiting on mwoehlke-kitware and tyler-yankee).

---

tools/workspace/github.bzl line 17 at r8 (raw file):

        tags_pattern = None,
        exclude_tags_pattern = None,
        upgrade_cooldown_days = 7,

nit This is optional, so should default to None. (The 7 should be defaulted in new_release.py, not here.)

---

tools/workspace/github.bzl line 231 at r8 (raw file):

        tags_pattern = getattr(repository_ctx.attr, "tags_pattern", None),
        exclude_tags_pattern = getattr(repository_ctx.attr, "exclude_tags_pattern", None),
        upgrade_cooldown_days = repository_ctx.attr.upgrade_cooldown_days,

nit The attribute should be truly optional, so that we don't need to change doxygen, jpeg, etc. Use getattr here like was done for commit_pin, etc.

---

tools/workspace/vtk_internal/repository.bzl line 158 at r8 (raw file):

        "upgrade_type": attr.string(),
        "tags_pattern": attr.string(),
        "exclude_tags_pattern": attr.string(),

nit I don't know why we need to provide {exclude_,}tags_pattern here at all? Maybe a rebase error?

[tyler-yankee]

@tyler-yankee reviewed 5 files and all commit messages, made 2 comments, and resolved 4 discussions.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform) (waiting on jwnimmer-tri).

---

tools/workspace/vtk_internal/repository.bzl line 158 at r8 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit I don't know why we need to provide {exclude_,}tags_pattern here at all? Maybe a rebase error?

(I added it for completeness per "These are the attributes for setup_github_repository." above, but I can remove it.)

---

tools/workspace/github.bzl line 223 at r9 (raw file):

    # Do the download step first.  (This also writes the metadata.)
    upgrade_cooldown_days = getattr(repository_ctx.attr, "upgrade_cooldown_days", -1)

@jwnimmer-tri We end up with a yucky workaround here because attr.int() can't be None. Is this fine, do you have any better ideas?

[jwnimmer-tri]

@jwnimmer-tri reviewed 4 files and all commit messages, made 3 comments, and resolved 1 discussion.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform) (waiting on tyler-yankee).

---

tools/workspace/github.bzl line 223 at r9 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

@jwnimmer-tri We end up with a yucky workaround here because attr.int() can't be None. Is this fine, do you have any better ideas?

This is probably the best we can do.

---

a discussion (no related file):
Maybe I'm reading the logic incorrectly, but I don't think this is what we want.

What we want is to choose the upgrade target without looking at dates -- the newest tag (but exclude prereleases), or the newest commit (tip of head), etc. Only after choosing that one single candidate, then we check the date. If the date of the single candidate is too-new, then we don't do anything.

What I believe the code currently does is step back through time until it finds some tag (or commit) that is sufficiently old, and than chooses that one. We don't want that because in case there was an older release with a severe bug, immediately followed by a patch releases that fixed it, we want to wait for the patch release. We never want to upgrade to a less-than-newest release.

---

tools/workspace/new_release.py line 188 at r9 (raw file):

    * it matches the given `exclude_pattern`
    * it seems to be a pre-release (ignored for lack of stability)
    * it is newer than one week (ignored for security reasons)

nit

Suggestion:

`cooldown_days`

[tyler-yankee]

@tyler-yankee made 1 comment.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform) (waiting on tyler-yankee).

---

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Maybe I'm reading the logic incorrectly, but I don't think this is what we want.

What we want is to choose the upgrade target without looking at dates -- the newest tag (but exclude prereleases), or the newest commit (tip of head), etc. Only after choosing that one single candidate, then we check the date. If the date of the single candidate is too-new, then we don't do anything.

What I believe the code currently does is step back through time until it finds some tag (or commit) that is sufficiently old, and than chooses that one. We don't want that because in case there was an older release with a severe bug, immediately followed by a patch releases that fixed it, we want to wait for the patch release. We never want to upgrade to a less-than-newest release.

Yes, this is correct. The "missing a slightly newer patch" idea did occur to me so thanks for bringing this up. Will address

[tyler-yankee]

As of the latest revision...

Skipping too-recent f50b1fd1e7b940f0c4e0c91d7e39fc0f15c45881 for common_robotics_utilities_internal
crate_universe may need upgrade
Skipping too-recent curl-8_21_0 for curl_internal
Skipping too-recent 1.0.43 for dm_control_internal
Skipping too-recent 4723170e1c8ee8bc59c342bb5c491ab8f263f1fb for drake_models
libjpeg_turbo_internal may need upgrade
Skipping too-recent f9dd4ccd798fe1791a8e44e417880ae31312d9a2 for meshcat
mujoco_menagerie_internal needs upgrade from 4c358ef9d9d7f32ca58b40b490884a0c1726a440 to accb6df40a9a1d1e49eff88157f6818b63a49335
nanoflann_internal needs upgrade from v1.10.0 to 1.10.1
qdldl_internal needs upgrade from v0.1.8 to v0.1.9
stable_baselines3_internal needs upgrade from v2.8.0 to v2.9.0
tinyobjloader_internal needs upgrade from 62ff207968f3dc14a64a1e2378dce67b760e7c4a to 45636bdcef1a4fec140346b90c0b50bf0bc3e23b
vtk_internal needs upgrade from 45f8cc6b6a4b14439ee3bab2025fa3ebeb20bfc0 to b4b7f93a1bea439074bd6b3dd69c2143ec7613f6
python may need upgrade but cannot be auto-upgraded.

I spot-checked a couple of those, and they are indeed within 7 days as of writing. Note that dm_control_internal had both 1.0.42 and 1.0.43, but we only try 1.0.43, unlike in my comment above.

@tyler-yankee reviewed 1 file and all commit messages, made 2 comments, and resolved 1 discussion.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform) (waiting on jwnimmer-tri).

---

a discussion (no related file):

Previously, tyler-yankee (Tyler Yankee) wrote…

Yes, this is correct. The "missing a slightly newer patch" idea did occur to me so thanks for bringing this up. Will address

Done.

[jwnimmer-tri]

Looks okay from a high level now. I'll let @mwoehlke-kitware do the first pass of review.

@jwnimmer-tri reviewed all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform) (waiting on tyler-yankee).

[mwoehlke-kitware]

@mwoehlke-kitware reviewed 66 files and all commit messages, and made 1 comment.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee jwnimmer-tri(platform) (waiting on tyler-yankee).

---

tools/workspace/new_release.py line 244 at r10 (raw file):

            continue
        return release.tag_name, release.published_at
    warn(f"Could not find any matching tags for {workspace}")

Suggestion:

releases

[mwoehlke-kitware]

with one minor nit

[tyler-yankee]

@tyler-yankee reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform) (waiting on tyler-yankee).

---

tools/workspace/new_release.py line 244 at r10 (raw file):

            continue
        return release.tag_name, release.published_at
    warn(f"Could not find any matching tags for {workspace}")

thanks!

[mwoehlke-kitware]

@mwoehlke-kitware reviewed 1 file and all commit messages.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform) (waiting on tyler-yankee).