upload: Allow arbitrary topic order

Fixes: #156
Skydio · Mar 12, 2024 · bc4eeaa · bc4eeaa
1 parent 09150f0
commit bc4eeaa
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 22 deletions.
diff --git a/docs/upload.md b/docs/upload.md
@@ -40,9 +40,7 @@ consecutive locally.
 : Optionally specifies a relative topic for this topic. Each topic can
 have at most one relative topic. Commits in this topic will be cherry-picked
 on top of the branch for the relative topic, and the pull request will
-be created targeted to the relative topic's branch. The first commit
-of a relative topic must appear before the first commit of specifying
-topic.
+be created targeted to the relative topic's branch.
 
 **Branches:**
 : Optionally specifies base branches for this topic. Base branches are long

diff --git a/revup/topic_stack.py b/revup/topic_stack.py
@@ -470,7 +470,8 @@ async def populate_reviews(
         Populate reviews for already-parsed topics. Verify base branch and relative topic info to
         ensure it is valid.
         """
-        seen_topics: Dict[str, Topic] = {}
+        last_topic = None
+        # Copy topics before iterating so it's safe to delete from the original dict
         for name, topic in list(self.topics.items()):
             if limit_topics:
                 if name not in limit_topics:
@@ -499,8 +500,8 @@ async def populate_reviews(
                 raise RevupUsageException(f"Can't specify more than one uploader for topic {name}!")
 
             relative_topic = ""
-            if force_relative_chain and seen_topics:
-                relative_topic = list(seen_topics)[-1]
+            if force_relative_chain and last_topic is not None:
+                relative_topic = last_topic
             elif len(topic.tags[TAG_RELATIVE]) > 1:
                 raise RevupUsageException(
                     "Can't specify more than 1 relative topic per topic! Got"
@@ -512,21 +513,12 @@ async def populate_reviews(
                 # all the base branches for the relative topic. However it can't specify
                 # any base branches the relative topic doesn't have.
                 relative_topic = min(topic.tags[TAG_RELATIVE])
-                if relative_topic not in seen_topics:
-                    if relative_topic in self.topics:
-                        # Relative topics can have interleaved commits, however the first commit of
-                        # the relative topic must come before the first commit of this topic. This
-                        # prevents cycles of relatives.
-                        raise RevupUsageException(
-                            f"Topic '{name}' is relative to '{relative_topic}' but doesn't appear"
-                            " after it"
-                        )
-                    else:
-                        logging.warning(
-                            f"Relative topic '{relative_topic}' not found in stack, assuming it was"
-                            " merged"
-                        )
-                        relative_topic = ""
+                if relative_topic not in self.topics:
+                    logging.warning(
+                        f"Relative topic '{relative_topic}' not found in stack, assuming it was"
+                        " merged"
+                    )
+                    relative_topic = ""
 
             if self.repo_info and self.fork_info and self.fork_info.owner != self.repo_info.owner:
                 if len(topic.tags[TAG_RELATIVE_BRANCH]) > 1:
@@ -561,7 +553,8 @@ async def populate_reviews(
             if auto_add_users in ("a2r", "both"):
                 topic.tags[TAG_REVIEWER].update(topic.tags[TAG_ASSIGNEE])
 
-            seen_topics[name] = topic
+            # Track the last actually used topic for the relative-chain feature
+            last_topic = name
 
         if limit_topics:
             for name in limit_topics: