added shutdown notice #170
Conversation
|
@hanbyul-here good idea. I'd like to suggest that we do this warning where the other warnings are made: https://github.com/mapzen/leaflet-geocoder/blob/master/src/core.js#L93 - and check to see if the It might also be a good idea to include a link to look for alternatives at the Pelias project: http://pelias.io/ |
src/core.js
Outdated
| // Make sure people aware of Mapzen hosted services are going down. | ||
| var mapzenHostedServiceUrl = 'https://search.mapzen.com'; | ||
|
|
||
| if (options.url.indexOf(mapzenHostedServiceUrl)) { |
There was a problem hiding this comment.
This will fail if options or options.url is undefined.
The other warnings will check to make sure options is defined (e.g. someone has actually specified options). It should also throw the warning if options.url isn't set, because it would then default to Mapzen Search.
There was a problem hiding this comment.
do you mind if I do check against this.options.url directly after default options were merged with users'? It seems to make more sense to check the final url set for this case.
|
That's also fine, I think!
…On Thu, Jan 4, 2018 at 12:09 PM, Hanbyul Jo ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/core.js
<#170 (comment)>
:
> @@ -101,6 +101,14 @@ var Geocoder = L.Control.extend({
'As of v1.6.0, the `latlng` option is deprecated. It has been renamed to `focus`. `latlng` will be removed in a future version.');
}
+ // Deprecation warnings for Mapzen hosted service.
+ // Make sure people aware of Mapzen hosted services are going down.
+ var mapzenHostedServiceUrl = 'https://search.mapzen.com';
+
+ if (options.url.indexOf(mapzenHostedServiceUrl)) {
do you mind if I do check against this.options directly after default
options were merged with users?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#170 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACb1tFJgbnlytlUOaquDLirdUJ7sO5vFks5tHQW8gaJpZM4RQ9OE>
.
|
|
wow @coveralls calm down. |
|
@hanbyul-here Looks great. I made this copy-edit change to the warning: BUT -- I no longer have push access to this repository. So I can't actually do any maintainance tasks, regrettably. I left a checklist for publishing here: https://github.com/mapzen/leaflet-geocoder/blob/master/CONTRIBUTING.md#versioning-and-publishing Not everything is relevant (e.g. updating dependencies and packages), but things like updating the version number and version numbers in demos should be done. I would recommend bumping this to a new minor version (so v1.10.0). |
|
I'm happy to re-take over the maintainance duties on this repository, but I'm not sure if it's worthwhile to restore my access to this or if it's better if I work on it in a fork. I think I still have access to the npm package, actually. cc @dianashk |
|
🤔 How do you think about moving this repo (or forking) to nextzen github and granting you access to that repo on nextzen? I am planning to do this for lrm-mapzen. |
|
Also thanks for copy editing! 🤗 |
|
@hanbyul-here Do it and let me know! |
|
well moving this repo to nextzen (and receiving commit permission) seems to cost :( I will merge this PR first we can talk more about where to moe this repo later. |
|
🎉 This PR is included in version 1.10.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Uh oh!
There was an error while loading. Please reload this page.