mapbox / mapbox-sdk-rb

A Ruby interface to Mapbox APIs.
Other
59 stars 32 forks source link

Add accounts tokens endpoints #43

Closed morgant closed 5 years ago

morgant commented 5 years ago

This adds support for the Tokens API (see Issue #41) by adding a Tokens class with the following methods: #list_tokens, #get_token, #create_token, #delete_token, #update_token, and #list_scopes.

It also modifies Mapbox#request to better support POST operations, incl. sending headers and JSONified parameters.

Documentation & tests are included, though tests are not currently mocked and actually create/update/delete tokens.

samfader commented 5 years ago

@morgant Thanks for your work on this? Do you know why Travis tests are failing?

morgant commented 5 years ago

@samfader My guess on the Travis tests failing is that the tests now require a MapboxUsername environment variable. I don't see any environment variables being set in .travis.yml, so assume that's something set on the admin side.

woodhull commented 5 years ago

As customers, we'd love to see this feature of the SDK or something like it merged officially. 🚀

samfader commented 5 years ago

@morgant Did a bit more research into the failing tests this morning. It looks like they are failing because your PR is coming from your forked repo. More here: https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions

How would you feel about making a PR directly from within this repo, rather than from your fork? While we can ignore the non-accounts related tests, the test you wrote are failing for the same reason as well, and I want to get those passing on here before merging.

If anyone knows of a better way to do this, let me know. I don't have a ton of experience with Travis builds nor a ton of time to dedicate to more research into this, so just trying to go with what seems like the simplest solution.

morgant commented 5 years ago

@samfader That does make sense for the cause.

It certainly makes sense to try creating a new branch on the main repo with the changes from my forked branch. You should be able to do so without my needing to be a contributor by treating it like merging an upstream repository into your fork, though checking out the new branch instead of master, as suggested in the GitHub article on the subject. Is that something you'd be willing to try? If so, thanks in advance.

samfader commented 5 years ago

https://github.com/mapbox/mapbox-sdk-rb/pull/44