Skip to content

Catch InvalidRepoUrlError and fail gracefully.#119

Merged
JohanLorenzo merged 2 commits intotaskcluster:mainfrom
kdashg:invalid-repo-url
Sep 22, 2022
Merged

Catch InvalidRepoUrlError and fail gracefully.#119
JohanLorenzo merged 2 commits intotaskcluster:mainfrom
kdashg:invalid-repo-url

Conversation

@kdashg
Copy link
Copy Markdown
Contributor

@kdashg kdashg commented Sep 21, 2022

This fixes mach bootstrap's InvalidRepoUrlError issues with a remote like "git@example.com:gecko".

Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1791852

This fixes `mach bootstrap`'s InvalidRepoUrlError issues with a remote
like "git@example.com:gecko".

Firefox bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1791852
@firefoxci-taskcluster
Copy link
Copy Markdown

No Taskcluster jobs started for this pull request
The `allowPullRequests` configuration for this repository (in `.taskcluster.yml` on the
default branch) does not allow starting tasks for this pull request.

@ahal ahal requested a review from JohanLorenzo September 21, 2022 21:02
@JohanLorenzo
Copy link
Copy Markdown
Contributor

Thank you very much for detailing the context and for providing this patch, @kdashg! At a first glance, it looks good to me. I'd like to add unit tests to ensure we don't regress that in the future. I see Taskcluster didn't run because of the policy on this repo (cc @ahal).

I was running the test suite when it actually enabled me to reproduce issues that other have had in the last comments of bug 1784232. That is such an awesome side effect of your PR so I would like to thank you again for helping me out 😃 . For the record, I documented how I got to repro in bug 1784232 comment 44.

If that's okay with you, I'll first debug what I repro then get back in this PR to add tests. Let me know what you think!

@ahal
Copy link
Copy Markdown
Collaborator

ahal commented Sep 22, 2022

I see Taskcluster didn't run because of the policy on this repo (cc @ahal).

Heh, this will be fixed when #113 lands (which is blocked on review for https://phabricator.services.mozilla.com/D157493)

@JohanLorenzo
Copy link
Copy Markdown
Contributor

JohanLorenzo commented Sep 22, 2022

If that's okay with you, I'll first debug what I repro then get back in this PR to add tests. Let me know what you think!

Status update: I managed to get my local clone in a better shape which allowed me to write tests. I opened #120 to get CI to run. If all goes well, I'll merge it 🙂

Copy link
Copy Markdown
Contributor

@JohanLorenzo JohanLorenzo left a comment

Choose a reason for hiding this comment

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

LGTM! All checks are green per https://github.com/taskcluster/taskgraph/runs/8495563748. Let's get this landed. Thanks again for your patch @kdashg! 👍

@JohanLorenzo JohanLorenzo merged commit 49d407c into taskcluster:main Sep 22, 2022
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.

3 participants