ci(milestone): give the next open milestone once the version's is closed

.github/workflows/milestone.yml

@@ -6,9 +6,10 @@ name: Milestone
 # that have none. The rule is util/milestone.py's action_for, documented under
 # "Milestone" in docs/source/contributing/workflows.rst: an open item, a merged
 # pull request or an issue closed as completed gets the open milestone named
-# after the checked-out `pcapkit.__version__`, unless it already holds another;
-# an unmerged pull request or an issue closed as not planned or as a duplicate
-# loses whichever milestone it holds. A missing milestone is a warning, never created.
+# after the checked-out `pcapkit.__version__`, or the lowest open one above it
+# once that one is closed (#2548), unless it already holds another; an unmerged
+# pull request or an issue closed as not planned or as a duplicate loses
+# whichever milestone it holds. A missing milestone is a warning, never created.
 #
 # SECURITY. `pull_request_target` gives this run a write token on events a
 # stranger can cause, so it must never execute anything the pull request

docs/source/contributing/conventions/process.rst

@@ -354,9 +354,13 @@ request, an issue closed as not planned -- takes no milestone.
 :file:`.github/workflows/milestone.yml` gives an item the open milestone as it opens,
 and settles it as the item closes; a milestone already set by hand to another
 release is kept, and a manual run backfills the items without one
-(:doc:`../workflows`). The closed milestones
-were backfilled from merge and close dates against the release tags, so an item that
-landed just before a tag may sit one line early.
+(:doc:`../workflows`). **The milestone follows the release, not** ``__version__``:
+after a final tag the version still names the release just made, so its closed
+milestone gives way to the lowest open one above it. The owner ruled that on
+:issue:`2548` over bumping ``__version__`` to a development release after each tag,
+which would block the scheduled releases until the suffix came off by hand. The closed
+milestones were backfilled from merge and close dates against the release tags, so an
+item that landed just before a tag may sit one line early.
 
 **The project board records state that the labels drop.** The state labels above are
 stripped when an item closes, so they say only what is live. The `PyPCAPKit project

docs/source/contributing/workflows.rst

@@ -597,7 +597,9 @@ closes, by the rule under "Milestones and the Project Board" in
 
 * an open item, a merged pull request, or an issue closed as completed gets the open
   milestone titled ``PyPCAPKit v<major>.<minor>`` from the checked-out
-  ``__version__`` -- ``1.5.0b14`` gives ``PyPCAPKit v1.5``;
+  ``__version__`` -- ``1.5.0b14`` gives ``PyPCAPKit v1.5`` -- or, once that one is
+  closed, the open ``PyPCAPKit v<major>.<minor>`` with the lowest release above it,
+  compared as numbers so ``v1.10`` follows ``v1.9`` (:issue:`2548`);
 * a pull request closed unmerged, or an issue closed as not planned or as a
   duplicate, loses any milestone it has;
 * an issue closed with no reason recorded is left as it is.
@@ -609,15 +611,15 @@ the exception: an unmerged or not-planned close takes away whichever milestone t
 item holds, since work closed without shipping belongs to no release.
 
 The run re-reads the item through the API rather than trusting the event payload.
-Where no open milestone has the title, the item is left without one and the run
-emits a ``::warning::``: the script never creates a milestone, since opening the
-next one is a release step done by hand. A removal needs no milestone, so it goes
-ahead regardless.
+Where no milestone has the title, or it is closed and none above it is open, the
+item is left without one and the run emits a ``::warning::``: the script never
+creates a milestone, since opening the next one is a release step done by hand. A
+removal needs no milestone, so it goes ahead regardless.
 
 A ``workflow_dispatch`` backfills: it applies the same rule to every item, open or
 closed, with no milestone whose number is above the highest-numbered item of the
 previous release's milestone, the newest of the ``PyPCAPKit v<major>.<minor>``
-milestones below the current one. Items at or below that bound belong to releases
+milestones below the one it gives. Items at or below that bound belong to releases
 already made, and those among them without a milestone were left so by hand. With
 no earlier milestone to bound it, the backfill warns and changes nothing. Its
 ``dry_run`` input is ticked by default, so a dispatch reports what it would change

tests/project/test_milestone_2218.py

@@ -3,6 +3,8 @@
 
 * :class:`TestTitle` and :class:`TestReadVersion` -- the milestone a version maps
   to, and the version read from :file:`pcapkit/__init__.py` as text.
+* :class:`TestTarget` -- the milestone given: the version's own while it is open,
+  else the lowest open one above it, by number (#2548).
 * :class:`TestActionFor` -- the rule: an open item, a merged pull request or an
   issue closed as completed gets the milestone; an unmerged one or one closed as
   not planned loses it; an issue closed with no reason is left alone.
@@ -155,6 +157,33 @@ def test_each_case(self) -> None:
 V16 = {'number': 12, 'title': 'PyPCAPKit v1.6', 'state': 'open'}
 V013 = {'number': 1, 'title': 'PyPCAPKit v0.13', 'state': 'closed'}
 OTHER = {'number': 11, 'title': 'Backlog', 'state': 'open'}
+#: v1.5 after its final tag, while ``__version__`` still names it (#2548).
+V15_CLOSED = dict(V15, state='closed')
+V17 = {'number': 13, 'title': 'PyPCAPKit v1.7', 'state': 'open'}
+V19_CLOSED = {'number': 20, 'title': 'PyPCAPKit v1.9', 'state': 'closed'}
+V110 = {'number': 21, 'title': 'PyPCAPKit v1.10', 'state': 'open'}
+V111 = {'number': 22, 'title': 'PyPCAPKit v1.11', 'state': 'open'}
+V20 = {'number': 23, 'title': 'PyPCAPKit v2.0', 'state': 'open'}
+
+
+class TestTarget(unittest.TestCase):
+    """#2548: the version's own milestone while it is open, else the lowest open one above it."""
+
+    def test_each_case(self) -> None:
+        cases = [
+            ('own open', '1.5.1', (V14, V15, V16, OTHER), V15),
+            ('own open, an earlier one open too', '1.5.1', (dict(V14, state='open'), V15, V16), V15),
+            ('own closed', '1.5.1', (V013, V14, V15_CLOSED, V16, OTHER), V16),
+            ('own closed, the lowest later one', '1.5.1', (V15_CLOSED, V17, V16), V16),
+            ('own closed, a later one closed too', '1.5.1', (V15_CLOSED, dict(V16, state='closed'), V17), V17),
+            ('own closed, only an earlier one open', '1.5.1', (dict(V14, state='open'), V15_CLOSED, OTHER), None),
+            ('own closed, none later', '1.5.1', (V14, V15_CLOSED, OTHER), None),
+            ('own absent, a later one open', '1.5.1', (V14, V16, OTHER), None),
+            ('v1.10 follows v1.9', '1.9.2', (V20, V111, V19_CLOSED, V110), V110),
+        ]
+        for case, version, found, want in cases:
+            with self.subTest(case=case):
+                self.assertEqual(milestone.target_milestone(list(found), version), want)
 
 
 class FakeRest:
@@ -203,8 +232,11 @@ def setUp(self) -> None:
         tmp = tempfile.TemporaryDirectory(prefix='pcapkit-2218-')  # pylint: disable=consider-using-with
         self.addCleanup(tmp.cleanup)
         self.version = os.path.join(tmp.name, '__init__.py')
+        self.write_version('1.5.0b14')
+
+    def write_version(self, version: 'str') -> None:
         with open(self.version, 'w', encoding='utf-8') as file:
-            file.write("__version__ = '1.5.0b14'\n")
+            file.write(f"__version__ = {version!r}\n")
 
     def run_main(self, argv: 'list[str]', fake: 'FakeRest') -> 'tuple[int, str]':
         #: The waits the run asked for, in order; nothing here really sleeps.
@@ -278,19 +310,40 @@ def test_a_reopened_item_gets_it_back_after_a_removal(self) -> None:
         self.assertEqual(fake.patches, [(372, None), (372, 10)])
 
     def test_a_missing_milestone_warns_and_changes_nothing(self) -> None:
-        for case, found in (('absent', (V013, V14, OTHER)),
-                            ('closed', (V013, V14, dict(V15, state='closed')))):
+        for case, found, why in (
+                ('absent', (V013, V14, OTHER),
+                 "no open milestone is titled 'PyPCAPKit v1.5', so no item was given one; create it by hand."),
+                ('closed, none later', (V013, V14, V15_CLOSED, OTHER),
+                 "'PyPCAPKit v1.5' is closed and no later release milestone is open, so no item was given one; "
+                 'create the next one by hand.')):
             with self.subTest(case=case):
                 fake = FakeRest([pull(2205, merged=True)], found=found)
                 code, out = self.run_main(['--item', '2205'], fake)
                 self.assertEqual(code, 0)
                 self.assertEqual(fake.patches, [])
-                self.assertEqual(out, "::warning title=Milestone::no open milestone is titled 'PyPCAPKit v1.5', "
-                                      'so no item was given one; create it by hand.\n0 of 1 item(s) changed\n')
+                self.assertEqual(out, f'::warning title=Milestone::{why}\n0 of 1 item(s) changed\n')
+
+    def test_a_closed_milestone_gives_the_next_open_one(self) -> None:
+        # #2548: 1.5.1 after v1.5's final tag, so its items go to v1.6.
+        self.write_version('1.5.1')
+        for case, item in (('issue opened', issue(2519, state='open', reason=None)),
+                           ('merged pull request', pull(2519, merged=True))):
+            with self.subTest(case=case):
+                fake = FakeRest([item], found=(V013, V14, V15_CLOSED, V17, V16, OTHER))
+                code, out = self.run_main(['--item', '2519'], fake)
+                self.assertEqual(code, 0)
+                self.assertEqual(fake.patches, [(2519, 12)])
+                self.assertEqual(out, "#2519: set milestone to 'PyPCAPKit v1.6'\n1 of 1 item(s) changed\n")
+
+    def test_the_next_release_is_found_numerically(self) -> None:
+        self.write_version('1.9.2')
+        fake = FakeRest([issue(900, state='open', reason=None)], found=(V20, V19_CLOSED, V111, V110))
+        _, out = self.run_main(['--item', '900'], fake)
+        self.assertEqual(fake.patches, [(900, 21)])
+        self.assertIn("#900: set milestone to 'PyPCAPKit v1.10'", out)
 
     def test_the_title_follows_the_version_file(self) -> None:
-        with open(self.version, 'w', encoding='utf-8') as file:
-            file.write("__version__ = '1.4.2'\n")
+        self.write_version('1.4.2')
         fake = FakeRest([issue(300)], found=(dict(V14, state='open'), V15))
         self.run_main(['--item', '300'], fake)
         self.assertEqual(fake.patches, [(300, 9)])
@@ -364,6 +417,22 @@ def test_a_missing_milestone_warns_and_changes_nothing(self) -> None:
         self.assertEqual(fake.patches, [])
         self.assertIn("::warning title=Milestone::no open milestone is titled 'PyPCAPKit v1.5'", out)
 
+    def test_a_closed_milestone_bounds_at_its_own_items(self) -> None:
+        # #2548: v1.6 is given, so v1.5 is the release before it, and #400 below
+        # v1.5's newest item was left without a milestone on purpose.
+        self.write_version('1.5.1')
+        items = [issue(400), issue(2501), pull(2502, merged=True), issue(2503, current=V16),
+                 issue(2504, state='open', reason=None)]
+        fake = FakeRest(items, found=(V013, V14, V15_CLOSED, V16, OTHER), newest={9: 336, 10: 2500, 1: 12})
+        code, out = self.run_main(['--backfill'], fake)
+        self.assertEqual(code, 0)
+        self.assertEqual(fake.patches, [(2501, 12), (2502, 12), (2504, 12)])
+        self.assertEqual(out.splitlines(), ['backfilling items above #2500 without a milestone',
+                                            "#2501: set milestone to 'PyPCAPKit v1.6'",
+                                            "#2502: set milestone to 'PyPCAPKit v1.6'",
+                                            "#2504: set milestone to 'PyPCAPKit v1.6'",
+                                            '3 of 3 item(s) changed'])
+
 
 class TestPrecedence(_Runs):
     """A milestone already on an item wins over the rule's, on open and on close, but not over a removal."""

util/milestone.py

@@ -6,7 +6,8 @@
 
 * an open item, a merged pull request, or an issue closed as completed gets the
   open milestone titled after the checked-out ``pcapkit.__version__``,
-  ``PyPCAPKit v<major>.<minor>`` (:func:`milestone_title`);
+  ``PyPCAPKit v<major>.<minor>`` (:func:`milestone_title`), or, where that one is
+  closed, the open release milestone lowest above it (:func:`target_milestone`);
 * a pull request closed unmerged, or an issue closed as not planned or as a
   duplicate, loses any milestone it has, whichever that is;
 * an issue closed with no reason recorded is left as it is.
@@ -16,9 +17,9 @@
 
 The version is read from :file:`pcapkit/__init__.py` as text (:func:`read_version`),
 so nothing from :mod:`pcapkit` is imported or run. Where the milestone for that
-version is missing or closed, an item that would get it is left alone and a
-``::warning::`` says so: creating the next milestone is a release step, done by
-hand.
+version is missing, or closed with no later one open, an item that would get it is
+left alone and a ``::warning::`` says so: creating the next milestone is a release
+step, done by hand.
 
 It runs in one of these modes:
 
@@ -27,7 +28,7 @@
   is re-read through the API rather than taken from the event payload, so a run
   applies the rule to the item as it stands when the run starts.
 * ``--backfill`` -- every item, open or closed, with no milestone whose number is
-  above the highest-numbered item of the previous release's milestone
+  above the highest-numbered item of the release milestone before the one given
   (:func:`backfill_bound`), for ``workflow_dispatch``. Items at or below that
   bound belong to releases already made, and the ones among them without a
   milestone were left without one on purpose.
@@ -247,9 +248,24 @@ def milestones(rest: 'Rest', repo: 'str') -> 'list[dict[str, Any]]':
     return list(pages(rest, f'repos/{repo}/milestones?state=all'))
 
 
-def find_open(found: 'list[dict[str, Any]]', title: 'str') -> 'Optional[dict[str, Any]]':
-    """The open milestone titled ``title`` among ``found``, if there is one."""
-    return next((entry for entry in found if entry['title'] == title and entry['state'] == 'open'), None)
+def target_milestone(found: 'list[dict[str, Any]]', version: 'str') -> 'Optional[dict[str, Any]]':
+    """The open milestone among ``found`` that a ``version``'s items get, if there is one.
+
+    It is the one :func:`milestone_title` names. Where that one is closed, its
+    release has shipped while ``__version__`` still names it, so it is the open
+    release milestone with the lowest release above it, compared as numbers so that
+    ``v1.10`` follows ``v1.9`` (#2548). A version with no milestone at all gets none.
+
+    """
+    title = milestone_title(version)
+    own = [entry for entry in found if entry['title'] == title]
+    current = next((entry for entry in own if entry['state'] == 'open'), None)
+    if current is not None or not own:
+        return current
+    floor = release(version)
+    later = [(number, entry) for entry in found if entry['state'] == 'open'
+             for number in [title_release(entry['title'])] if number is not None and number > floor]
+    return min(later, key=lambda pair: pair[0])[1] if later else None
 
 
 def backfill_bound(rest: 'Rest', repo: 'str', found: 'list[dict[str, Any]]',
@@ -336,21 +352,28 @@ def main(argv: 'Optional[list[str]]' = None, rest: 'Rest' = gh_rest,
         items = [rest('GET', f'repos/{args.repo}/issues/{args.item}', None)]
         actions = [action_for(items[0])]
         found = milestones(rest, args.repo) if SET in actions else []
+        milestone = target_milestone(found, version)
     else:
         found = milestones(rest, args.repo)
-        bound = backfill_bound(rest, args.repo, found, release(version))
+        milestone = target_milestone(found, version)
+        # Bounded by the release before the milestone given, not before the version's.
+        given = title if milestone is None else milestone['title']
+        bound = backfill_bound(rest, args.repo, found, release(given[len(PREFIX):]))
         if bound is None:
-            _actions.annotate('warning', f'no release milestone before {title!r}, so nothing bounds the '
+            _actions.annotate('warning', f'no release milestone before {given!r}, so nothing bounds the '
                               'backfill; no item was changed.', title=ANNOTATION)
             return 0
         print(f'backfilling items above #{bound} without a milestone')
         items = unmilestoned(rest, args.repo, bound)
         actions = [action_for(item) for item in items]
 
-    milestone = find_open(found, title)
     if milestone is None and SET in actions:
-        _actions.annotate('warning', f'no open milestone is titled {title!r}, so no item was given one; '
-                          'create it by hand.', title=ANNOTATION)
+        if any(entry['title'] == title for entry in found):
+            message = (f'{title!r} is closed and no later release milestone is open, so no item was given one; '
+                       'create the next one by hand.')
+        else:
+            message = f'no open milestone is titled {title!r}, so no item was given one; create it by hand.'
+        _actions.annotate('warning', message, title=ANNOTATION)
 
     wrote = False