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

Validate topic names #103

Merged
merged 1 commit into from
Apr 5, 2023
Merged

Validate topic names #103

merged 1 commit into from
Apr 5, 2023

Conversation

jerry-skydio
Copy link
Collaborator

@jerry-skydio jerry-skydio commented Apr 4, 2023

Catch the common case of someone sticking an invalid char in the topic
line.

Topic: catch1-./Q
Reviewers: brian-k

@jerry-skydio
Copy link
Collaborator Author

Reviews in this chain:
#103 Validate topic names

@jerry-skydio
Copy link
Collaborator Author

jerry-skydio commented Apr 4, 2023

# head base diff date summary
0 9736a989 fa4e5a9a diff Apr 4 2:21 AM 1 file changed, 4 insertions(+)
1 23ec53a2 fa4e5a9a diff Apr 4 3:02 AM 0 files changed
2 9736a989 fa4e5a9a diff Apr 4 7:31 AM 0 files changed
3 2e2dd003 1a86af93 diff Apr 4 19:01 PM 0 files changed
4 e844f7d8 1a86af93 diff Apr 4 19:02 PM 0 files changed
5 5fa08e87 2cd01f01 diff Apr 4 20:02 PM 1 file changed, 46 insertions(+), 23 deletions(-)
6 e45db27c 2cd01f01 diff Apr 4 20:07 PM 1 file changed, 1 insertion(+), 1 deletion(-)

@jerry-skydio jerry-skydio force-pushed the jerry/revup/main/catch1-./Q branch 2 times, most recently from 23ec53a to 9736a98 Compare April 4, 2023 14:32
@@ -54,6 +54,8 @@ def format_remote_branch(uploader: str, base_branch: str, topic: str) -> str:

RE_COMMIT_LABEL = re.compile(r"^(?P<label1>[a-zA-Z\-_0-9]+):.*|^\[(?P<label2>[a-zA-Z\-_0-9]+)\].*")

RE_BRANCH_ALLOWED = re.compile(r"^[a-zA-Z\d_\.\/-]+$")

Choose a reason for hiding this comment

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

\w instead of a-zA-Z\d so unicode can be used

Also, comment on where this restriction comes from (valid branch names?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm i haven't tested with unicode but let me try that

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(commented)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm github doesn't seem super happy but it did work #106 so i'll go with this

@jerry-skydio jerry-skydio force-pushed the jerry/revup/main/catch1-./Q branch 2 times, most recently from 2e2dd00 to e844f7d Compare April 5, 2023 02:02
Catch the common case of someone sticking an invalid char in the topic
line.

Topic: catch1-./Q
Reviewers: brian-k
@jerry-skydio jerry-skydio force-pushed the jerry/revup/main/catch1-./Q branch 2 times, most recently from 5fa08e8 to e45db27 Compare April 5, 2023 03:07
@jerry-skydio jerry-skydio merged commit 932120e into main Apr 5, 2023
@jerry-skydio jerry-skydio deleted the jerry/revup/main/catch1-./Q branch April 5, 2023 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants