Update feature service to add layer(s) &| table(s) #317
Conversation
| import { IUserRequestOptions } from "@esri/arcgis-rest-auth"; | ||
| import { ILayer, ITable } from "@esri/arcgis-rest-common-types"; | ||
|
|
||
| export interface IAddToFeatureServiceRequestOptions |
There was a problem hiding this comment.
If you're going to call the method updateFeatureService(), i'd recommend using IUpdateFeatureService... as interface names too.
There was a problem hiding this comment.
Good point; how about we rename the function to addToFeatureService?
There was a problem hiding this comment.
@noahmulfinger drafted some function names i quite liked back in October 2017 based on his team's work in https://developers.arcgis.com/.
do you have any misgivings about the way he laid things out?
There was a problem hiding this comment.
Good resource--thanks!
| }, | ||
| response => { | ||
| // We're not interested in the full ArcGISRequestError response, nor having an exception thrown | ||
| resolve(response.response); |
There was a problem hiding this comment.
i don't quite grok your code comment. what is it exactly that you're going for here? specifically why you don't want devs to catch() errors?
There was a problem hiding this comment.
@jgravois, the underlying REST call sends an HTTP 200 for success or failure, but the request call converts the failure into an exception. Purpose of this section is to 1) align with style of REST call and 2) reduce the wordy content returned by failed request call; much of that content simply echoes what the app sent. Not needed by app, and more to define in error response type.
There was a problem hiding this comment.
the request call converts the failure into an exception.
Thats actually an intentional design choice in this API. We think developers will consider it more intuitive to trust that everything went according to plan if a promise resolves and catch() errors from our REST API (even if they were sent with a 200 status code).
There was a problem hiding this comment.
OK; easy enough to change
|
@jgravois, I think that I addressed your review comments. |
|
i pushed up another tweak that further simplifies how errors are handled. please take a look at 31e56db and let me know what you think. |
|
Sounds good, @jgravois; thanks |
|
@MikeTschudi if it'd be helpful to you for us to publish this to npm, just say the word. |
|
@jgravois, what does publishing to npm do, please? |
|
npm is a package manager for sharing JS code that gets more popular by the day. it includes a cli that allows folks to pull down any bundle they're interested in. https://www.npmjs.com/package/@esri/arcgis-rest-feature-service-admin usually when folks use npm, they organize the dependencies for their project in a arcgis-rest-js/demos/attachments/package.json Lines 8 to 11 in d3f2553 i offered to publish a release to npm because the version thats up there right now has the method you contributed to create feature services, but it doesn't yet include the method you added for modifying them. |
|
we gotta get me back on the release wagon. I'm gunshy after the releases I made didn't bump all the packages. Maybe @jgravois can hold my hand on Mon while I run the scripts? |
|
Thanks, @jgravois. I've been using @esri/... without thinking about how we get from the repository to @esri/... |
No description provided.