Skip to content
This repository has been archived by the owner on Jan 6, 2023. It is now read-only.

Update Search samples to newest geocode locator #1075

Merged
merged 18 commits into from
Jun 15, 2021
Merged

Conversation

vquach2404
Copy link
Collaborator

This PR updates all of the samples in the Search category affected by common-samples/issues/2246. The offline data for Offline geocode has been updated and the links to the geocoding service have been updated for all other samples.

I updated the commenting style and made small improvements to the coding style (not meant to be an entire revamp of the source code).

@vquach2404 vquach2404 requested a review from yo1995 May 27, 2021 00:34
@vquach2404 vquach2404 marked this pull request as ready for review May 27, 2021 00:39
Copy link
Collaborator

@yo1995 yo1995 left a comment

Choose a reason for hiding this comment

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

A few things that I notice

  • In the Find place sample, the results seem to be sparse. I think Tanner mentioned similar stuff so I'll treat it as expected
  • The coding style changes are interweaving in the comment changes, so I found it bit hard to tell if they would cause new problems. But ad-hoc testing makes me believe these 4 samples still work fine.
  • Not sure if we should also update the READMEs. Some are referencing this service: https://www.arcgis.com/home/item.html?id=305f2e55e67f4389bef269669fc2e284 which should be OK, just want to double check.

Co-authored-by: Ting Chen <tchen@esri.com>
@vquach2404
Copy link
Collaborator Author

This link is preferred over the actual service link because the portal item provides helpful information about the service and also provides a link to the service link.

@vquach2404
Copy link
Collaborator Author

  • In the Find place sample, the results seem to be sparse. I think Tanner mentioned similar stuff so I'll treat it as expected

I've reached out to Christa who helped us put this data together. She's on vacation at the moment so I'll convert this to a draft in the meantime.

@vquach2404 vquach2404 marked this pull request as draft June 2, 2021 23:16
@vquach2404 vquach2404 marked this pull request as ready for review June 8, 2021 19:27
@vquach2404
Copy link
Collaborator Author

I've reached out to Christa who helped us put this data together. She's on vacation at the moment so I'll convert this to a draft in the meantime.

Christa has confirmed that the sample is behaving as expected. This PR is ready for review!

@vquach2404 vquach2404 requested a review from yo1995 June 8, 2021 19:29
yo1995
yo1995 previously approved these changes Jun 8, 2021
Co-authored-by: Philip Ridgeway <pridgeway@esri.com>
philium
philium previously approved these changes Jun 14, 2021
Copy link
Contributor

@saratchandrakarumuri saratchandrakarumuri left a comment

Choose a reason for hiding this comment

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

New data in search samples is working as expected

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants