Skip to content

Initial commit of Vimeo module - #278

Merged
MrSwitch merged 1 commit into
MrSwitch:masterfrom
jimmed:vimeo-module
Jul 2, 2015
Merged

Initial commit of Vimeo module#278
MrSwitch merged 1 commit into
MrSwitch:masterfrom
jimmed:vimeo-module

Conversation

@jimmed

@jimmed jimmed commented Jul 2, 2015

Copy link
Copy Markdown

This is the hello.js counterpart to MrSwitch/node-oauth-shim#15

@MrSwitch
MrSwitch merged commit 7965c59 into MrSwitch:master Jul 2, 2015
@jimmed
jimmed deleted the vimeo-module branch July 2, 2015 15:10
@jimmed

jimmed commented Jul 2, 2015

Copy link
Copy Markdown
Author

Thanks for including this! Would it be possible to update the npm version of hello.js with these changes? Cheers!

@MrSwitch

MrSwitch commented Jul 2, 2015

Copy link
Copy Markdown
Owner

@jimmed

jimmed commented Jul 3, 2015

Copy link
Copy Markdown
Author

Thanks very much, and thanks for providing such a helpful library!

@jimmed

jimmed commented Jul 3, 2015

Copy link
Copy Markdown
Author

Sorry to be a pain, but it seems that v1.7.0 hasn't been re-built with the Vimeo module included - there's no reference to vimeo in dist/hello.all.js

@MrSwitch

MrSwitch commented Jul 3, 2015

Copy link
Copy Markdown
Owner

@jimmed sorry about the lack of communication there. I removed it from the list because i didn't think it had the minimum set of required features.

Importantly the facade for querying Vimeo needs to have some predefined endpoints. So that it behaves like all other HelloJS modules. These endpoints even if they already match need to be defined, so we can test for their presence. This helps populate the pages like http://adodson.com/hello.js/tests/#hello-x-api-path--method--data--callback--

i.e.

...
get : {
    me: 'me',
    me/followers: 'me/followers?per_page=@{limit|50}'
}
...

Also converts the generic hello js parameter, i.e. limit, start.. etc. I think Vimeos equivalent to is per_page and page respectively.

Also the response should be formatted to match the expected response. These differ per endpoint, again please see //adodson.com/hello.js/tests/ examples.

I hope you'll consider working on this some more to get it ready. It seems like a pretty comprehensive API they have.

@MrSwitch

MrSwitch commented Jul 3, 2015

Copy link
Copy Markdown
Owner

btw: in the meantime, you can still include src/modules/vimeo.js as a separate script after loading any dist/hello.*.

@jimmed

jimmed commented Jul 3, 2015

Copy link
Copy Markdown
Author

Unfortunately I'm working in a webpack bundle so including the file directly isn't a particularly pleasant option. I have a functional workaround for now so I'm happy, but I don't think I'll have a large amount of time to commit to fleshing out the rest of the module. Watch this space ⬇️

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.

2 participants