Update Stamen providers to reflect that they are now hosted by Stadia Maps

[ianthetechie]

Pretty much what the title says. This change is non-breaking (the current Fastly URLs have been redirecting here for a few weeks already), and reflects that registration will soon be required. I have not touched any of the code structure, so things should keep working for existing users of the library.

[brunob]

related to #519

[brunob]

Really nice PR, thx for that !

[martinfleis]

Shouldn't these be variants of Stadia maps instead? Rather than having them separately as Stamen as it was until now. Strictly speaking, they are becoming just variants of Stadia tiles.

[ianthetechie]

@martinfleis technically speaking, yes, it would make sense for these to go under Stadia. Would you prefer to do this as a breaking change and do a 2.0.0 release?

[martinfleis]

This is a breaking change anyway no? Since this PR changes variant names from toner to stamen_toner.

I find the solution of filing these under Stadia better from a long-term perspective but this is a bit specific situation.

What about including them under Stadia but leave around the original Stamen urls until October? As a form of transition period.

[ianthetechie]

Yeah, I struggled with that... Is it a semver breaking change if the public API remains the same, but an external dependency has a cutoff date? That's tricky... I would not suggest leaving the current URLs for the transition period. The old URLs will completely stop working on October 31st, but we will start doing "brownouts" (serving tiles letting you know you have to switch) starting in September.

All that said, I think I see your point about labeling it as a breaking change anyway and moving under Stadia. It will make it more explicit that something is changing.

Re: the current changes being breaking, I think the construction from the public API is the same, no? I thought the mapping to stamen_toner was an internal detail, but I admit I am not that familiar with this codebase :D

L.tileLayer.provider('Stamen.Toner').addTo(map);

[martinfleis]

the current changes being breaking, I think the construction from the public API is the same, no?

I guess you are right, it is not breaking change as is. Sorry!

[ianthetechie]

Any further thoughts from @brunob?

[brunob]

Shouldn't these be variants of Stadia maps instead?

This should be that, but it will force us to bump a v2 since it'll cause "the breaking change".

I would not suggest leaving the current URLs for the transition period

+1

Maybe we can try to add aliases for old Stamen calls to new ones from Stadia in order to avoir the v2, but i think it's simpler to assume the change and directly bump a v2. Any thoughts about this @jieter ?

[ianthetechie]

I just went ahead and pushed the "breaking" version (v2 moving everything under Stadia) since it seems like that's the way things will land.

[jieter]

Maybe we can try to add aliases for old Stamen calls to new ones from Stadia in order to avoir the v2, but i think it's simpler to assume the change and directly bump a v2. Any thoughts about this @jieter ?

I'd say immediate v2 will be the lowest maintenance cost, which we should prefer I think.

[brunob]

I'd say immediate v2 will be the lowest maintenance cost, which we should prefer I think.

So we can merge this one and push a new release :)

[ianthetechie]

Cool cool! FWIW I couldn't figure out how to run unit tests locally (but did test that it looks good on the preview page) and it looks like actions have been failing somewhat often recently. I didn't break anything new as far as I can tell ;)

[ianthetechie]

Hey, just a friendly bump on getting this PR merged @brunob. Anything I can do to help speed up a release.

We originally planned to start serving a certain proportion of "warning tiles" letting users on the old endpoints to upgrade starting in early September, but would prefer if the major libraries supported the new URLs first.

[jieter]

I see no reason not to merge, other than the failing CI. But that seems unrelated to this change, I'll fix those after merging.

[brunob]

@jieter before pushing to npm, i think we should complete the changelog 1.13.0...master

[brunob]

@jieter before pushing to npm, i think we should complete the changelog 1.13.0...master

done #524