Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Github Automation: Add support for an optional colon after command name #66540

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

PranavReddyP16
Copy link

As requested in #64803

example -> /cherry-pick: or /branch:

execute_command function (example -> /cherry-pick:)
Copy link
Collaborator

@tstellar tstellar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the regex is correct, I'm fine with this.

@PranavReddyP16
Copy link
Author

Assuming the regex is correct, I'm fine with this.

I tested it with a lot of sample strings and it worked so I think it's correct.

"""
for line in sys.stdin:
line.rstrip()
m = re.search(r"/([a-z-]+)\s(.+)", line)
m = re.search(r"/([a-z-]+)(?::)?\s(.+)", line)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

Suggested change
m = re.search(r"/([a-z-]+)(?::)?\s(.+)", line)
m = re.search(r"/([a-z-]+):?\s(.+)", line)

Then the reader doesn't have to remember the rules for (?:...)s

@nikic
Copy link
Contributor

nikic commented Sep 22, 2023

I think after #66781 this will no longer work.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 22, 2023

@llvm/pr-subscribers-github-workflow

Changes

As requested in #64803

example -> /cherry-pick: or /branch:


Full diff: https://github.com/llvm/llvm-project/pull/66540.diff

1 Files Affected:

  • (modified) llvm/utils/git/github-automation.py (+3-3)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index eac5816b5499f6a..090359b8822a73a 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -572,12 +572,12 @@ def execute_command(self) -> bool:
         """
         This function reads lines from STDIN and executes the first command
         that it finds.  The 2 supported commands are:
-        /cherry-pick commit0 <commit1> <commit2> <...>
-        /branch <owner>/<repo>/<branch>
+        /cherry-pick<:> commit0 <commit1> <commit2> <...>
+        /branch<:> <owner>/<repo>/<branch>
         """
         for line in sys.stdin:
             line.rstrip()
-            m = re.search(r"/([a-z-]+)\s(.+)", line)
+            m = re.search(r"/([a-z-]+)(?::)?\s(.+)", line)
             if not m:
                 continue
             command = m.group(1)

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 22, 2023

@llvm/pr-subscribers-github-workflow

Changes

As requested in #64803

example -> /cherry-pick: or /branch:


Full diff: https://github.com/llvm/llvm-project/pull/66540.diff

1 Files Affected:

  • (modified) llvm/utils/git/github-automation.py (+3-3)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index eac5816b5499f6a..090359b8822a73a 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -572,12 +572,12 @@ def execute_command(self) -> bool:
        """
        This function reads lines from STDIN and executes the first command
        that it finds.  The 2 supported commands are:
-        /cherry-pick commit0 <commit1> <commit2> <...>
-        /branch <owner>/<repo>/<branch>
+        /cherry-pick<:> commit0 <commit1> <commit2> <...>
+        /branch<:> <owner>/<repo>/<branch>
        """
        for line in sys.stdin:
            line.rstrip()
-            m = re.search(r"/([a-z-]+)\s(.+)", line)
+            m = re.search(r"/([a-z-]+)(?::)?\s(.+)", line)
            if not m:
                continue
            command = m.group(1)

Error: Command failed due to missing milestone.

1 similar comment
@llvmbot
Copy link
Collaborator

llvmbot commented Sep 22, 2023

@llvm/pr-subscribers-github-workflow

Changes

As requested in #64803

example -> /cherry-pick: or /branch:


Full diff: https://github.com/llvm/llvm-project/pull/66540.diff

1 Files Affected:

  • (modified) llvm/utils/git/github-automation.py (+3-3)
diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py
index eac5816b5499f6a..090359b8822a73a 100755
--- a/llvm/utils/git/github-automation.py
+++ b/llvm/utils/git/github-automation.py
@@ -572,12 +572,12 @@ def execute_command(self) -> bool:
        """
        This function reads lines from STDIN and executes the first command
        that it finds.  The 2 supported commands are:
-        /cherry-pick commit0 <commit1> <commit2> <...>
-        /branch <owner>/<repo>/<branch>
+        /cherry-pick<:> commit0 <commit1> <commit2> <...>
+        /branch<:> <owner>/<repo>/<branch>
        """
        for line in sys.stdin:
            line.rstrip()
-            m = re.search(r"/([a-z-]+)\s(.+)", line)
+            m = re.search(r"/([a-z-]+)(?::)?\s(.+)", line)
            if not m:
                continue
            command = m.group(1)

Error: Command failed due to missing milestone.

@PranavReddyP16
Copy link
Author

@tbaederr given #66781, do we still want to continue with this? Tagging since @tbaederr opened the issue for this (#64803)

@tbaederr
Copy link
Contributor

Yes? The colon would still work if it's followed by a space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Needs Triage
Development

Successfully merging this pull request may close these issues.

None yet

8 participants