gdrive support #833
gdrive support #833
Conversation
There was a problem hiding this comment.
Looks awesome, @Maxris ! Please, check some comments to address.
|
@shcheklein @efiop updated, thanks! |
There was a problem hiding this comment.
Looks good to me! Pending @jorgeorpinel 's review before we merge.
| ❗Do not share Google Drive access token with anyone to avoid unauthorized usage | ||
| of your Google Drive. |
There was a problem hiding this comment.
Continuation of #833 (review)
But is it DVC's responsibility to alert about G Drive security? If not, maybe just a note (md quote starting with >) will suffice after all @shcheklein?
Regardless, please use this text:
Please remember that Google Drive access tokens are personal credentials and should not be shared with anyone, otherwise risking unauthorized usage of the Google Drive.
There was a problem hiding this comment.
I think it's good to have this for GD - it not a common knowledge with all these tokens involved into GD.
There was a problem hiding this comment.
quotes with > are not visible. they defeat the purpose to some extent for notes like this. I'm fine with > though. I don't like when we mix > with some Note! or something -they look not very clean to me.
But tbh - I'm fine with all of these option. Jorge, I think we'll let you decide on this :)
There was a problem hiding this comment.
I agree notes with > are not visible and perhaps we shouldn't mix with bold "Note!". That's kind of contradictory! Opened #848 for this.
in this case though, while I like having the note, I do see it as a side-note that could be completely omitted and not something that DVC can really control. So for that reason and since we don't really have emojis ATM, I'm going to take it back to a > note...
Should we open an issue to apply some basic emoji symbols throughout with some consistency though? This could be a nice improvement to our docs.
|
@jorgeorpinel @Maxris hey guys, besides the consistency in order point is there anything else that is blocking this from merging? |
|
@shcheklein hopefully, no others blocking things |
|
@jorgeorpinel have merged this, let's address the order consistency (a minor issue anyways) as a separate ticket. |
|
Yeah thanks, I was a little out of pocket last week. I'll just address the small details myself in a quick PR. Soon |
Fix #381
gdrive).client idandclient secretconfig options forgdriveremote.