Skip to content

Commit

Permalink
upload: Add "branch-format" config option and commit tag
Browse files Browse the repository at this point in the history
Branch-format will let you control how branches are named,
which has an effect on when branches collide. This can be
set on cmdline, from config file, or on a topic-by-topic
basis inside the commit text.

user+branch is the default today and will remain the default.
Branch names will never collide if using this, but you cannot
retarget to other base branches, and you must use Uploader: if
another user wants to push to this branch.

user allows retargeting prs to other base branches, but will
not allow multiple base-branches to be specified. if they are,
it will warn to specify user+branch instead.

branch will allow others to upload to the same pr, but uploader:
cannot be used.

none includes both limitations and features of user and branch.

I don't really believe there's good use cases for branch and none,
but i'm implementing them to be complete.
  • Loading branch information
jerry-skydio committed Mar 13, 2024
1 parent 8c17f14 commit 051b8f7
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 7 deletions.
13 changes: 11 additions & 2 deletions docs/upload.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ multiple users to collaborate on the same PR. If uploader is specified, all
relative topics must specify the same uploader. However a topic without a
specified uploader can still be relative to a topic with one.

**Branch-Format:**
: Specifies how the remote branches get named, which mainly affects how names
conflict. Default is "user+branch", which never conflicts, but does not allow
retargeting a PR to a different base branch. "user" will allow retargeting, but
will not allow multiple base branches. "branch" and "none" are also supported.This tag takes precedence over the config option.

**Relative-Branch:**
: Optionally specifies a relative branch that this review is targeted against.
A relative branch is one that represents another user's work or PR, and is
Expand Down Expand Up @@ -114,10 +120,13 @@ branch as the base. See above section for definition of a base branch.
definition of a relative branch.

**--uploader**
: Used as the username for naming remote branches. The branch syntax
is {uploader}/revup/{basebranch}/{topic}. If not set value is taken
: Used as the username for naming remote branches. If not set value is taken
from the portion of "git config user.email" before the "@".

**--branch-format**
: Specify how branches are named. See the Branch-Format: tag section for
options and their meaning.

**--rebase, -r**
: By default revup will not push changes if local commits are a pure
rebase of the remote changes. This flag overrides that behavior and causes
Expand Down
3 changes: 3 additions & 0 deletions revup/revup.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,9 @@ async def main() -> int:
"--user-aliases",
)
upload_parser.add_argument("--uploader")
upload_parser.add_argument(
"--branch_format", choices=["user+branch", "user", "branch", "none"], default="user+branch"
)
upload_parser.add_argument("--pre-upload", "-p")
upload_parser.add_argument("--relative-chain", "-c", action="store_true")
upload_parser.add_argument("--auto-topic", "-a", action="store_true")
Expand Down
41 changes: 36 additions & 5 deletions revup/topic_stack.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,23 @@
# https://docs.github.com/en/get-started/using-git/dealing-with-special-characters-in-branch-and-tag-names
RE_BRANCH_ALLOWED = re.compile(r"^[\w\d_\.\/-]+$")

BRANCH_FORMAT_STRATEGIES = {
"user+branch": "{uploader}/revup/{base_branch}/{topic}",
"user": "{uploader}/revup/{topic}",
"branch": "revup/{base_branch}/{topic}",
"none": "revup/{topic}",
}


def format_remote_branch(uploader: str, base_branch: str, topic: str) -> str:
def format_remote_branch(uploader: str, base_branch: str, topic: str, branch_format: str) -> str:
"""
Branches are named so that it is clear that they are made by revup
and can be force pushed at any time, and to minimize collision with
manually created branches.
"""
return f"{uploader}/revup/{base_branch}/{topic}"
return BRANCH_FORMAT_STRATEGIES[branch_format].format(
uploader=uploader, base_branch=base_branch, topic=topic
)


RE_TAGS = re.compile(r"^(?P<tagname>[a-zA-Z\-]+):(?P<tagvalue>.*)$", re.MULTILINE)
Expand All @@ -49,6 +58,7 @@ def format_remote_branch(uploader: str, base_branch: str, topic: str) -> str:
TAG_RELATIVE_BRANCH = "relative-branch"
TAG_UPLOADER = "uploader"
TAG_UPDATE_PR_BODY = "update-pr-body"
TAG_BRANCH_FORMAT = "branch-format"
VALID_TAGS = {
TAG_BRANCH,
TAG_LABEL,
Expand All @@ -59,6 +69,7 @@ def format_remote_branch(uploader: str, base_branch: str, topic: str) -> str:
TAG_TOPIC,
TAG_UPLOADER,
TAG_UPDATE_PR_BODY,
TAG_BRANCH_FORMAT,
}

RE_COMMIT_LABEL = re.compile(r"^(?P<label1>[a-zA-Z\-_0-9]+):.*|^\[(?P<label2>[a-zA-Z\-_0-9]+)\].*")
Expand Down Expand Up @@ -504,6 +515,15 @@ async def populate_reviews(
f"Invalid tags for update-pr-body: {topic.tags[TAG_UPDATE_PR_BODY]}"
)

if TAG_BRANCH_FORMAT in topic.tags:
if (
len(topic.tags[TAG_BRANCH_FORMAT]) > 1
or min(topic.tags[TAG_BRANCH_FORMAT]).lower() not in BRANCH_FORMAT_STRATEGIES
):
raise RevupUsageException(
f"Invalid tags for branch-format: {topic.tags[TAG_BRANCH_FORMAT]}"
)

relative_topic = ""
if force_relative_chain and last_topic is not None:
relative_topic = last_topic
Expand Down Expand Up @@ -573,7 +593,7 @@ async def populate_reviews(
if name not in self.topics:
logging.warning(f"Couldn't find any topic named {name}")

async def populate_relative_reviews(self, uploader: str) -> None:
async def populate_relative_reviews(self, uploader: str, branch_format: str) -> None:
for name, topic in self.topological_topics():
if topic.relative_topic:
if len(topic.tags[TAG_BRANCH]) == 0:
Expand Down Expand Up @@ -638,7 +658,16 @@ async def populate_relative_reviews(self, uploader: str) -> None:
f" {topic.relative_topic.tags[TAG_UPLOADER] or '{}'}"
)
topic_uploader = min(topic.tags[TAG_UPLOADER]) if topic.tags[TAG_UPLOADER] else uploader

topic_branch_format = (
min(topic.tags[TAG_BRANCH_FORMAT]).lower()
if TAG_BRANCH_FORMAT in topic.tags
else branch_format
)
if "branch" not in topic_branch_format and len(topic.tags[TAG_BRANCH]) > 1:
raise RevupUsageException(
"Cannot use multiple branches if Branch-Format: does not "
'include "branch", try overriding it for this topic'
)
for branch in topic.tags[TAG_BRANCH]:
review = Review(topic)
# Track whether we need to query for the relative pr
Expand All @@ -663,7 +692,9 @@ async def populate_relative_reviews(self, uploader: str) -> None:
review.base_ref = await self.git_ctx.to_commit_hash(relative_branch)
review.remote_base = self.git_ctx.remove_branch_prefix(relative_branch)

review.remote_head = format_remote_branch(topic_uploader, base_branch, name)
review.remote_head = format_remote_branch(
topic_uploader, base_branch, name, topic_branch_format
)

topic.reviews[branch] = review

Expand Down
1 change: 1 addition & 0 deletions revup/upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ async def main(
)
await topics.populate_relative_reviews(
args.uploader if args.uploader else git_ctx.author,
branch_format=args.branch_format,
)

if not args.dry_run:
Expand Down

0 comments on commit 051b8f7

Please sign in to comment.