-
Notifications
You must be signed in to change notification settings - Fork 37
Remove github.com requirement #197
Comments
What are your thoughts on this? |
I think it'd be better to just verify that the repo host is github before making the call to the API |
will throw together a PR tomorrow. |
This might be tricky since you'll have to parse the |
Can't we just add a regex for github.com? |
I think that sets a bad precedent if we want to add logic based on the host |
I think this will actually be kind of tricky now that you bring it up. What are your thoughts on how to proceed? |
I think a good first step would be to see how janky extracting the host from the |
wouldn't we be able to just run: git remote -v This would display all of the hosts, we could split by Note: |
I think it'd be much easier to just parse the output of |
We could always check (although, a prerequisite to sail is git) |
git remote -v sounds good to me
…On Fri, May 24, 2019 at 6:11 AM Robert M ***@***.***> wrote:
We could always check (although, a prerequisite to sail is git)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#197>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABQJ7BZOFBRIQQQ2AYVHIRDPW7EONANCNFSM4HPKE6KQ>
.
|
So, what do you think that the solution should be? Should we have a regex for the remotes? If there is a GitHub* remote, we should use it. Is that an ideal solution? Although, this solution is not very good since you can have a remote that looks like |
I think just parse the output of |
ok, I will put together a PR together when I get sail to work on my mac (since I am away from my desktop). |
@kami1019 Not sure what your problem is... Can you elaborate a little bit? |
Currently, there is a GitHub SDK within the code. If there is no
.sail/Dockerfile
, the basecodercom/ubuntu-dev
image should be used.The text was updated successfully, but these errors were encountered: