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.

Sign up for GitHub

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

Jump to bottom

companion: rename microsoft and google options to onedrive and drive respectively #2346

Merged
merged 2 commits into from
Jul 6, 2020
Merged

companion: rename microsoft and google options to onedrive and drive respectively #2346

merged 2 commits into from
Jul 6, 2020

Conversation

ifedapoolarewaju
Copy link
Contributor

This helps minimise the inconsistency, as the providers are originally referred to as onedrive and drive (for Google Drive), it's confusing that you have to set options for microsoft and google instead.

ifedapoolarewaju
ifedapoolarewaju commented Jun 29, 2020
View reviewed changes
packages/@uppy/companion-client/src/Provider.js
@@ -12,7 +12,6 @@ module.exports = class Provider extends RequestClient {
super(uppy, opts)
this.provider = opts.provider
this.id = this.provider
this.authProvider = opts.authProvider || this.provider
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realised that this is not used anywhere

@goto-bus-stop
Copy link
Contributor

goto-bus-stop commented Jun 29, 2020 •
edited
Loading

could we add a console warning if you pass in options for the old name? or throw an error even.

@arturi
Copy link
Contributor

arturi commented Jun 29, 2020

So it’s different in src/standalone/helper.js and src/config/grant.js 🤔

@ifedapoolarewaju
Copy link
Contributor Author

So it’s different in src/standalone/helper.js and src/config/grant.js 🤔

Yes, src/config/grant.js is an internal requirement/usage. It was the source of confusion to begin with. But tomorrow we can decide not want to use grant.js anymore, and that shouldn't break usage of Companion. So this change (and some previous others) are attempts at making User provided options independent of Companion's internals.

@ifedapoolarewaju ifedapoolarewaju mentioned this pull request Jun 29, 2020
Merged
@arturi arturi self-requested a review July 3, 2020 08:24
@ifedapoolarewaju
Copy link
Contributor Author

could we add a console warning if you pass in options for the old name? or throw an error even.

@goto-bus-stop I have made this update, thank you

@ifedapoolarewaju ifedapoolarewaju mentioned this pull request Jul 6, 2020
Closed
@ifedapoolarewaju ifedapoolarewaju merged commit 772e54c into master Jul 6, 2020
@delete-merged-branch delete-merged-branch bot deleted the rename-microsoft-opt branch July 6, 2020 13:22
This was referenced Jan 16, 2022
Open
Open
Open
Open
@H4NG-MAN H4NG-MAN mentioned this pull request Sep 25, 2022
Open
@snyk-bot snyk-bot mentioned this pull request Sep 26, 2022
Open
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
@ifedapoolarewaju
companion: rename microsoft and google options to onedrive and drive …
95fa26c 
…respectively (transloadit#2346)

* companion: rename microsoft and google  provider options to onedrive and drive respectively

* companion: throw error when the deprecated option is set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arturi arturi arturi approved these changes

@goto-bus-stop goto-bus-stop goto-bus-stop approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ifedapoolarewaju @goto-bus-stop @arturi