forge: Genericize github handling into a forge abstraction

README.md

@@ -76,7 +76,7 @@ git clone https://github.com/<your-name>/revup.git && cd revup
 On first run, you will need to configure github credentials. Create a [GitHub Personal Access Token](https://github.com/settings/tokens/new?scopes=repo)
 with "repo" scope. Revup needs this in order to create and modify pull requests. Then run
 ```sh
-revup config github_oauth
+revup config forge_oauth
 ```
 and copy and paste the oauth into the prompt.
 

docs/revup.md

@@ -10,7 +10,7 @@ revup - Efficient git workflow and code review toolkit
 
 Revup is a python command-line toolkit for speeding up your
 git workflow. It provides commit-based development support and
-full github integration.
+full forge integration (GitHub, with more backends planned).
 
 All revup options can be given with a shorter unambiguous prefix.
 
@@ -25,7 +25,7 @@ line, the value of the last one will be used.
 : Prints out extra details for debugging. This will include the
 full command-line of any subprocesses that are run along with
 their full output, as well as the full input and output of any
-graphql requests to github.
+graphql requests to the forge.
 
 **--help, -h**
 : Show this help page.
@@ -34,32 +34,29 @@ graphql requests to github.
 : Print the version of revup and exit.
 
 **--proxy**
-: Proxy to use when making connections to GitHub
+: Proxy to use when making connections to the forge.
 
-**--github-oauth**
-: The oauth token that provides login credentials to github. Revup
+**--forge-oauth**
+: The oauth token that provides login credentials to the forge. Revup
 requires full repository read/write permissions in order to create
-and modify reviews. This is represented by the "repo" section of
+and modify reviews. For GitHub, this is represented by the "repo" section of
 https://github.com/settings/tokens/new.
 
-**--github-username**
-: The user's github username for login.
-
-**--github-url**
-:  URL to use for github. Defaults to "github.com" and would only
-need changed if the user is using github enterprise.
+**--forge-url**
+: URL to use for the forge. Defaults to "github.com" and would only
+need changed if the user is using GitHub Enterprise or another host.
 
 **--remote-name**
-: The name of the remote that corresponds to github. Branches on this
+: The name of the remote that corresponds to the forge. Branches on this
 remote are also used for base branch detection. Defaults to "origin".
 
 **--fork-name**
-: If specified, the name of the remote that corresponds to a github fork
+: If specified, the name of the remote that corresponds to a fork
 that should be used to push branches to. The pull request will be created
 using the branch from this fork. If empty, remote-name is used for both
 pushing and creating the pull request.
 
-Github does not allow base branches of pull requests to be in a different
+GitHub does not allow base branches of pull requests to be in a different
 fork, so reviews with a Relative: label will be deferred until its base
 merges. Relative-Branch cannot be used across forks.
 
@@ -114,7 +111,7 @@ cache. Can also change the commit text.
 given branch relative to its base branch, then cherry-pick it.
 
 **revup upload**
-: Upload and push the current stack of code reviews to github.
+: Upload and push the current stack of code reviews.
 
 **revup restack**
 : Reorder the current stack so that commits in a topic are consecutive.

revup/__main__.py

@@ -6,7 +6,7 @@
 from revup.revup import build_parser, main
 from revup.types import (
     RevupConflictException,
-    RevupGithubException,
+    RevupForgeException,
     RevupRequestException,
     RevupShellException,
     RevupUsageException,
@@ -35,8 +35,8 @@ def _main() -> None:
     except RevupShellException as e:
         logging.error(str(e))
         sys.exit(4)
-    except RevupGithubException as e:
-        logging.error(f"Github Exception: {e.type}: {e.message}")
+    except RevupForgeException as e:
+        logging.error(f"Forge Exception: {e.type}: {e.message}")
         sys.exit(5)
     except RevupRequestException as e:
         logging.error(f"Request failed with response status {e.status}")

revup/amend.py

@@ -160,8 +160,6 @@ async def get_has_unstaged() -> bool:
         git_ctx,
         args.base_branch,
         args.relative_branch,
-        None,
-        None,
     )
     if args.ref_or_topic:
         commit = await parse_ref_or_topic(args.ref_or_topic, args, git_ctx, topics)

revup/cherry_pick.py

@@ -3,12 +3,7 @@
 from typing import Tuple
 
 from revup import config, git
-from revup.github_utils import (
-    RE_PR_URL,
-    github_connection,
-    parse_pull_request_url,
-    query_pr_by_number,
-)
+from revup.forge_utils import RE_PR_URL, forge_connection, parse_pull_request_url
 from revup.types import GitCommitHash, GitTreeHash, RevupUsageException
 
 
@@ -19,13 +14,9 @@ async def resolve_pr_url(
     Resolve a PR URL to (head_ref, base_ref) by querying the GitHub API.
     """
     pr_params = parse_pull_request_url(pr_url)
-    async with github_connection(args=args, git_ctx=git_ctx, conf=conf) as (
-        github_ep,
-        _repo_info,
-        _fork_info,
-    ):
-        head_ref, base_ref = await query_pr_by_number(
-            github_ep, pr_params.owner, pr_params.name, pr_params.number
+    async with forge_connection(args=args, git_ctx=git_ctx, conf=conf) as forge:
+        head_ref, base_ref = await forge.query_pr_by_number(
+            pr_params.owner, pr_params.name, pr_params.number
         )
     logging.info(f"Resolved PR #{pr_params.number} to branch '{head_ref}' based on '{base_ref}'")
     return head_ref, base_ref

revup/config.py

@@ -71,9 +71,10 @@ def get_actions(self) -> Dict[str, argparse.Action]:
                 # Ignore nonconfigurable actions (help, auto-generated negation)
                 continue
 
-            if len(action.option_strings) > 0 and action.option_strings[0].startswith("--"):
-                option = action.option_strings[0][2:].replace("-", "_")
-                ret[option] = action
+            for opt in action.option_strings:
+                if opt.startswith("--"):
+                    option = opt[2:].replace("-", "_")
+                    ret.setdefault(option, action)
         return ret
 
     def set_option_default(self, option: str, action: argparse.Action, value: str) -> None:
@@ -209,7 +210,7 @@ def config_main(conf: Config, args: argparse.Namespace, all_parsers: List[RevupA
             )
     elif args.value:
         value = args.value
-        if command == "revup" and key == "github_oauth":
+        if command == "revup" and key in ("forge_oauth", "github_oauth"):
             logging.warning(
                 "Prefer to omit the value on command line when entering sensitive info. "
                 "You may want to clear your shell history."
@@ -226,15 +227,7 @@ def config_main(conf: Config, args: argparse.Namespace, all_parsers: List[RevupA
         # (this may throw if the value is not allowed)
         parser.set_option_default(key, actions[key], value)
 
-        if command == "revup" and key == "github_username":
-            # From https://www.npmjs.com/package/github-username-regex :
-            # Github username may only contain alphanumeric characters or hyphens.
-            # Github username cannot have multiple consecutive hyphens.
-            # Github username cannot begin or end with a hyphen.
-            # Maximum is 39 characters.
-            if not re.match(r"^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$", value, re.I):
-                raise ValueError(f"{value} is not a valid GitHub username")
-        elif command == "revup" and key == "github_oauth":
+        if command == "revup" and key in ("forge_oauth", "github_oauth"):
             if not re.match(r"^[a-z\d_]+$", value, re.I):
                 raise ValueError("Input string is not a valid oauth")
 

revup/forge.py

@@ -0,0 +1,131 @@
+from abc import ABCMeta, abstractmethod
+from dataclasses import dataclass, field
+from typing import Dict, List, Optional, Set, Tuple
+
+
+@dataclass
+class ForgeRepoInfo:
+    name: str = ""
+    owner: str = ""
+
+
+@dataclass
+class PullRequestParams:
+    forge_url: str
+    owner: str
+    name: str
+    number: int
+
+
+@dataclass
+class PrComment:
+    text: str = ""
+    id: Optional[str] = None
+
+
+@dataclass
+class PrInfo:
+    baseRef: str
+    headRef: str
+    baseRefOid: Optional[str]
+    headRefOid: Optional[str]
+    body: str
+    title: str
+    id: str = ""
+    url: str = ""
+    state: str = ""
+    reviewers: Set[str] = field(default_factory=set)
+    reviewer_ids: Set[str] = field(default_factory=set)
+    reviewer_teams: Set[str] = field(default_factory=set)
+    reviewer_team_ids: Set[str] = field(default_factory=set)
+    assignees: Set[str] = field(default_factory=set)
+    assignee_ids: Set[str] = field(default_factory=set)
+    labels: Set[str] = field(default_factory=set)
+    label_ids: Set[str] = field(default_factory=set)
+    removed_reviewers: Set[str] = field(default_factory=set)
+    removed_reviewer_ids: Set[str] = field(default_factory=set)
+    removed_assignees: Set[str] = field(default_factory=set)
+    removed_assignee_ids: Set[str] = field(default_factory=set)
+    is_draft: bool = False
+    comments: List[PrComment] = field(default_factory=list)
+
+
+@dataclass
+class PrUpdate:
+    baseRef: Optional[str] = None
+    body: Optional[str] = None
+    title: Optional[str] = None
+    id: str = ""
+    reviewer_ids: Set[str] = field(default_factory=set)
+    reviewer_team_ids: Set[str] = field(default_factory=set)
+    assignee_ids: Set[str] = field(default_factory=set)
+    label_ids: Set[str] = field(default_factory=set)
+    is_draft: Optional[bool] = None
+    comments: List[PrComment] = field(default_factory=list)
+
+
+MAX_COMMENTS_TO_QUERY = 3
+
+
+class Forge(metaclass=ABCMeta):
+    @property
+    def name(self) -> str:
+        return type(self).__name__.lower()
+
+    @property
+    @abstractmethod
+    def repo_owner(self) -> str:
+        """Owner of the fork remote (or repo remote if no fork)."""
+
+    @property
+    @abstractmethod
+    def repo_name(self) -> str:
+        """Name of the repository."""
+
+    @property
+    @abstractmethod
+    def is_fork(self) -> bool:
+        """True if the fork remote points to a different owner than the repo remote."""
+
+    @abstractmethod
+    async def query_everything(
+        self,
+        head_refs: List[str],
+        user_ids: List[str],
+        labels: List[str],
+        teams: List[Tuple[str, str]],
+    ) -> Tuple[
+        str,
+        List[Optional[PrInfo]],
+        Dict[str, str],
+        Dict[str, str],
+        Dict[str, str],
+        Dict[str, str],
+        Dict[str, Optional[Set[str]]],
+    ]:
+        """
+        Query all needed info in one request. Returns:
+        - Repository node id
+        - List of pull requests (None if not found for that ref)
+        - Dict of user query strings to node ids
+        - Dict of user query strings to full login names
+        - Dict of label names to node ids
+        - Dict of team refs ("org/slug") to node ids
+        - Dict of team refs to member logins (None if membership unknown)
+        """
+
+    @abstractmethod
+    async def create_pull_requests(self, repo_id: str, prs: List[PrInfo]) -> None:
+        """Create pull requests. Modifies prs in-place to set id and url."""
+
+    @abstractmethod
+    async def update_pull_requests(self, prs: List[PrUpdate]) -> None:
+        """Update existing pull requests."""
+
+    @abstractmethod
+    async def query_pr_by_number(self, owner: str, name: str, number: int) -> Tuple[str, str]:
+        """Query a pull request by number and return (headRefName, baseRefName)."""
+
+    @abstractmethod
+    async def close(self) -> None:
+        """Clean up any connections."""