New arcgis-rest-features package w/ getFeature() to get a feature b…

[tomwayson]

…y id

begins to address #46

Started w/ getFeature() just to wire up the new package and run the names by you as I shortened what I proposed in #46 (comment) from getFeatureById() to getFeature() b/c that seemed more in line w/ getItem().

[coveralls]

Coverage remained the same at 100.0% when pulling 1d0e57e on feat/get-feature into 78af91f on master.

[tomwayson]

Oh, and @patrickarlt - I can't run the docs site, even on master, so this PR doesn't include any changes (if any) that are needed over there. It seems to hang on docs:typedoc:

➜  arcgis-rest-js git:(master) npm run docs:serve 

> @esri/arcgis-rest-js@1.0.2 predocs:serve /Users/tom/code/arcgis-rest-js
> npm run docs:typedoc


> @esri/arcgis-rest-js@1.0.2 docs:typedoc /Users/tom/code/arcgis-rest-js
> node docs/build-typedoc.js


Using TypeScript 2.4.1 from /Users/tom/code/arcgis-rest-js/node_modules/typedoc/node_modules/typescript/lib

[jgravois]

because getFeature() always requires at least two pieces of information, my gut reaction is that this should be

getFeature({
  url: serviceUrl,
  id: 42,
  session?: IUserSession
} as IFeatureRequestOptions)

see #78

[tomwayson]

good point @jgravois - I was just working off the spec we laid out in #46 which predated the one arg to rule them all discussion in #78. I forgot about that. I'll update accordingly.

[noahmulfinger]

@tomwayson What's the reasoning for naming the package arcgis-rest-feature-service without the s? It seems like arcgis-rest-feature-services is more consistent with arcgis-rest-items and arcgis-rest-groups

Also, I think package-lock.json should get added to this PR.

[tomwayson]

Good question @noahmulfinger.

When I stared on this package, I thought a lot about the name. The first conclusion I came to was that I don't think we should avoid appending -service where possible, the exception being the package you are planning to actually administer services.

So, I was thinking that the best name for this package, and the best way to be consistent w/ arcgis-rest-items and arcgis-rest-groups would be arcgis-rest-features, but I worried that getLayerInfo or getServiceInfo would not fit well under that name. I'm glad to go back to arcgis-rest-features if y'all think that's not a problem.

Once I made the name about the service rather than the features, the only other peer out of the other pacakges (-auth, -common-types, -geocoder, -request) was really arcgis-rest-geocoder, which is kinda it's own thing. So no real help there.

Also, back in #46 you and I both flip flopped back and forth btw/ arcgis-rest-feature-service and arcgis-rest-feature-services - you originally proposed the singular, then we settled on the latter. However, now that we have -items and -groups, I thought the singular would best.

[noahmulfinger]

@tomwayson Okay, thinking about it more, arcgis-rest-feature-service sounds better to me as well. I kinda think going singular with all the packages would be better, but we can think about in the future.

[tomwayson]

Thanks @noahmulfinger. I'm still torn, I like arcgis-rest-features. I'm still worried that we're going to end up w/ -service on a lot of package names.

A way we could standardize on non-pulral names would have been:
arcgis-rest-items -> arcgis-rest-content
arcgis-rest-groups -> arcgis-rest-sharing

[noahmulfinger]

@tomwayson Yeah, I see what you mean. However, I don't think we will need to append -service to any of the services that represent specific actions or tasks, for instance arcgis-rest-geoenrichment, arcgis-rest-spatial-analysis, arcgis-rest-geometry.

And, at least in this case, I think the full term feature-service is necessary to have getServiceInfo() and getLayerInfo(), as you mentioned in the issue.

[tomwayson]

Good point.

As per a discussion w/ @jgravois we're going to hold on merging this until he can cut a release w/ the fixes since 1.0.2 since this will require a minor bump.

Also, FYI I've got getLayerInfo and queryFeatures queued up locally and will push PRs for those once this gets merged.