[tools] Stylistic cleanups to new_release

[tyler-yankee]

Provide type hints, except for github3 objects which don't provide them. Establish enum classes for some types that previously used constants or ad-hoc strings. Remove some unused arguments, and make the order of arguments more consistent.

Requires #24593.

---

This change is 

[tyler-yankee]

+a:@mwoehlke-kitware for feature review, please. (I only marked the PR "medium" priority because of the small bug w.r.t. crate_universe from the last PR.)

@tyler-yankee reviewed 2 files 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]

-(priority: medium) I've pulled out the crate_universe fix into #24613.

@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).

[tyler-yankee]

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

[mwoehlke-kitware]

@mwoehlke-kitware partially reviewed 1 file and made 5 comments.
Reviewable status: 5 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 114 at r1 (raw file):

    @property
    def is_github(self) -> bool:
        return self in [RuleType.GITHUB, RuleType.GITHUB_RELEASE_ATTACHMENTS]

BTW, is there a reason to use a list here rather than a set?

---

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

def _latest_tag(
    gh_repo, workspace: str, exclude_pattern: str | None = None

gh_repo is (should be) a github3.repos.Repository?

---

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

def _handle_github(
    gh, workspace_name: str, data: dict[str, Any]

gh is (should be) a github3.GitHub?

---

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

            "-o", *paths, "-m", "[workspace] " + message
        )
        info("")

Uh... I don't feel great about removing these. They make it much easier to pick out important text, and even if we're 100% certain that there is no unimportant text, it's still useful as an indicator that the user needs to pay attention.

---

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

def _do_commit(
    local_drake_checkout,

local_drake_checkout is (should be) a git.Repo?

[tyler-yankee]

@tyler-yankee made 4 comments.
Reviewable status: 5 unresolved discussions, 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 114 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

BTW, is there a reason to use a list here rather than a set?

No preference either way.

---

tools/workspace/new_release.py line 182 at r1 (raw file):
From the commit message:

Provide type hints, except for github3 objects which don't provide them

Context: the github3 package does not provide .pyi files, hinting them is useless other than arguably readability for the "vanilla" text (is that what you're proposing?)

---

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

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Uh... I don't feel great about removing these. They make it much easier to pick out important text, and even if we're 100% certain that there is no unimportant text, it's still useful as an indicator that the user needs to pay attention.

Yeah, I agree with not removing them either; they've just been factored out to above and below theif/else.

---

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

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

local_drake_checkout is (should be) a git.Repo?

I probably just missed this one, thanks!

[mwoehlke-kitware]

@mwoehlke-kitware made 2 comments and resolved 1 discussion.
Reviewable status: 4 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 182 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

From the commit message:

Provide type hints, except for github3 objects which don't provide them

Context: the github3 package does not provide .pyi files, hinting them is useless other than arguably readability for the "vanilla" text (is that what you're proposing?)

I guess. If I was doing this, I'd write the hints, even if they "aren't enforceable".

---

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

Previously, tyler-yankee (Tyler Yankee) wrote…

Yeah, I agree with not removing them either; they've just been factored out to above and below theif/else.

Ah, I missed the addition at the beginning of the function. (At the end of the function is well hidden; since it's only whitespace removal, it wasn't obvious.)

[tyler-yankee]

@tyler-yankee reviewed 1 file and all commit messages, made 2 comments, and resolved 3 discussions.
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 182 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

I guess. If I was doing this, I'd write the hints, even if they "aren't enforceable".

Fair - done!

---

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

Previously, tyler-yankee (Tyler Yankee) wrote…

I probably just missed this one, thanks!

done

[mwoehlke-kitware]

@mwoehlke-kitware made 3 comments.
Reviewable status: 2 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 114 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

No preference either way.

👍
It probably doesn't matter for a collection this size, but as a matter of practice, there's an argument for using sets being good practice.

---

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

Previously, tyler-yankee (Tyler Yankee) wrote…

done

There are additional instances of local_drake_checkout...

---

-- commits line 6 at r3:
BTW, it seems inconsistent using ; elsewhere but , here. Since none of the items use , internally, maybe just replace ; with ,?

[tyler-yankee]

@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 mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on mwoehlke-kitware).

---

-- commits line 6 at r3:

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

BTW, it seems inconsistent using ; elsewhere but , here. Since none of the items use , internally, maybe just replace ; with ,?

done, and just simplified the language a little

---

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

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

There are additional instances of local_drake_checkout...

Sorry about that; fixed.

[mwoehlke-kitware]

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

[tyler-yankee]

+a:@jwnimmer-tri for platform review, please.

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

[jwnimmer-tri]

@jwnimmer-tri reviewed 2 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 187 at r4 (raw file):

) -> str | None:
    """Returns the latest tag for the given `workspace` that doesn't match an
    ignore rule."""

nit "an ignore rule" seems needlessly imprecise given that we have a name for it (exclude_pattern), or it's at least a pattern not a rule. (Using different names for the same thing begs the question of whether they are supposed to be different concepts or not.)

[tyler-yankee]

@tyler-yankee reviewed 1 file 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 187 at r4 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

nit "an ignore rule" seems needlessly imprecise given that we have a name for it (exclude_pattern), or it's at least a pattern not a rule. (Using different names for the same thing begs the question of whether they are supposed to be different concepts or not.)

This is a good point, but I'd argue:

• in _is_ignored_tag we should be more explicit as you say ("exclude pattern")
• here in _latest_tag we should keep "ignore rule" because it corresponds to whatever _is_ignored_tag says, which could be pre-release, or after #24570 could be a too-recent release.

[tyler-yankee]

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

[jwnimmer-tri]

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

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),mwoehlke-kitware (waiting on tyler-yankee).

[mwoehlke-kitware]

@mwoehlke-kitware reviewed 1 file and all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),mwoehlke-kitware (waiting on tyler-yankee).