sail icon indicating copy to clipboard operation
sail copied to clipboard

Remove github.com requirement

Open teddy-codes opened this issue 6 years ago • 17 comments

Currently, there is a GitHub SDK within the code. If there is no .sail/Dockerfile, the base codercom/ubuntu-dev image should be used.

teddy-codes avatar May 23 '19 20:05 teddy-codes

/cc @nathanpotter @ammario

What are your thoughts on this? Re: #196

teddy-codes avatar May 23 '19 20:05 teddy-codes

I think it'd be better to just verify that the repo host is github before making the call to the API

nathanpotter avatar May 23 '19 22:05 nathanpotter

will throw together a PR tomorrow.

teddy-codes avatar May 24 '19 00:05 teddy-codes

This might be tricky since you'll have to parse the .git folder in the project for the host

coadler avatar May 24 '19 01:05 coadler

Can't we just add a regex for github.com?

teddy-codes avatar May 24 '19 01:05 teddy-codes

I think that sets a bad precedent if we want to add logic based on the host

coadler avatar May 24 '19 01:05 coadler

I think this will actually be kind of tricky now that you bring it up.

What are your thoughts on how to proceed?

teddy-codes avatar May 24 '19 01:05 teddy-codes

I think a good first step would be to see how janky extracting the host from the .git folder would be

coadler avatar May 24 '19 01:05 coadler

wouldn't we be able to just run:

git remote -v

This would display all of the hosts, we could split by \n, then use the endpoints there?

Note: This would mean that if any of the repositories were on GitHub, this would happen. Although, it is likely that if it is, then there is no problem.

teddy-codes avatar May 24 '19 01:05 teddy-codes

.git/config is where git stores all of the remotes in a format that seems ini-ish:

...
[remote "origin"]
        url = [email protected]:cdr/sail.git
        fetch = +refs/heads/*:refs/remotes/origin/*
...

I think it'd be much easier to just parse the output of git directly as roberthmiller pointed out, as if they're working with Git in the first place it's very likely that they have git installed on their system.

deansheather avatar May 24 '19 07:05 deansheather

We could always check (although, a prerequisite to sail is git)

teddy-codes avatar May 24 '19 11:05 teddy-codes

git remote -v sounds good to me

On Fri, May 24, 2019 at 6:11 AM Robert M [email protected] 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 https://github.com/cdr/sail/issues/197?email_source=notifications&email_token=ABQJ7BY3UFHBCLLOPZZR363PW7EONA5CNFSM4HPKE6K2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWE62OY#issuecomment-495578427, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQJ7BZOFBRIQQQ2AYVHIRDPW7EONANCNFSM4HPKE6KQ .

coadler avatar May 24 '19 14:05 coadler

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 git:cdr/sail and it would be aliased as github.com in the ssh config. What are your thoughts on this?

teddy-codes avatar May 24 '19 22:05 teddy-codes

I think just parse the output of git remote -v if git is exists, and if it has a URL beginning with https://github.com/ or [email protected]: then try to get the language from GitHub. If people have aliased GitHub to something else it won't work, but I don't think many people do that anyways.

deansheather avatar May 25 '19 07:05 deansheather

ok, I will put together a PR together when I get sail to work on my mac (since I am away from my desktop).

teddy-codes avatar May 25 '19 14:05 teddy-codes

@kami1019 Not sure what your problem is... Can you elaborate a little bit?

teddy-codes avatar Jun 20 '19 15:06 teddy-codes