<p></p>
<p><b>@gravitystorm</b> commented on this pull request.</p>
<p>I realise that this is going upstream, and for the variable naming etc they will have their own preferences. But here's a small review anyway, and I think the url changes are worth highlighting.</p><hr>
<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/2584#discussion_r406659831">vendor/assets/leaflet/leaflet.osm.js</a>:</p>
<pre style='color:#555'>> @@ -1,8 +1,25 @@
L.OSM = {};
+var h2 = false;
</pre>
<p>I'd choose <code>http2</code> as the variable name, for clarity. I think <code>h2</code> is a bit obscure.</p>
<hr>
<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/2584#discussion_r406683619">vendor/assets/leaflet/leaflet.osm.js</a>:</p>
<pre style='color:#555'>> @@ -1,8 +1,25 @@
L.OSM = {};
+var h2 = false;
+// tile.openstreetmap.org supports http/2 where sharding makes no sense.
+// Use only one subdomain if we're using http/2
+if (
+ (window.performance && window.performance.getEntries && performance.getEntries()[0].nextHopProtocol == 'h2') ||
+ (window.performance && performance.timing.nextHopProtocol && performance.timing.nextHopProtocol == 'h2') ||
+ (window.chrome && window.chrome.loadTimes && window.chrome.loadTimes().connectionInfo == 'h2')
+) {
+ h2 = true;
+}
+
+if (h2) {
+ var osmtileurl = 'https://a.tile.openstreetmap.org/{z}/{x}/{y}.png';
</pre>
<p>I recommend using <code>tile.openstreetmap.org</code> if you don't need the abc subdomains, since the <code>a</code> is just an artifact of the load distribution that this patch moves away from.</p>
<hr>
<p>In <a href="https://github.com/openstreetmap/openstreetmap-website/pull/2584#discussion_r406683723">vendor/assets/leaflet/leaflet.osm.js</a>:</p>
<pre style='color:#555'>> @@ -44,9 +61,14 @@ L.OSM.HOT = L.OSM.TileLayer.extend({
}
});
+if(h2) {
+ var osmgpsurl = 'https://gps-a.tile.openstreetmap.org/lines/{z}/{x}/{y}.png';
</pre>
<p>As above - <code>gps.tile.openstreetmap.org</code> is sufficient</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/2584#pullrequestreview-391330068">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AAK2OLJLSB3TE5VQLKGY2WDRL3SMFANCNFSM4ME55M2A">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AAK2OLPDQFDQULETNIN26PTRL3SMFA5CNFSM4ME55M2KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOC5JTSFA.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/openstreetmap/openstreetmap-website/pull/2584#pullrequestreview-391330068",
"url": "https://github.com/openstreetmap/openstreetmap-website/pull/2584#pullrequestreview-391330068",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>