Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.2.0.beta changelog #8529

Merged
merged 1 commit into from
Jul 23, 2019
Merged

1.2.0.beta changelog #8529

merged 1 commit into from
Jul 23, 2019

Conversation

peterqliu
Copy link
Contributor

@peterqliu peterqliu commented Jul 22, 2019

No description provided.

Copy link
Contributor

@asheemmamoowala asheemmamoowala left a comment

Choose a reason for hiding this comment

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

This is a good start, but I think a lot of items have snuck in from the v1.1.1 release, but also a couple from older ones.

Could also use more developer-friendly entries than just the titles from the corresponding PRs/commits

CHANGELOG.md Outdated
* Add map-specific access tokens ([#8364](https://github.com/mapbox/mapbox-gl-js/pull/8364))
* Get start-docs working on linux ([#8486](https://github.com/mapbox/mapbox-gl-js/pull/8486))
* Add *-sort-key layout property for circle, fill, line ([#8467](https://github.com/mapbox/mapbox-gl-js/pull/8467))
* Include Hiragana and Katakana code blocks for local glyph generation ([#8365](https://github.com/mapbox/mapbox-gl-js/pull/8365))
Copy link
Contributor

Choose a reason for hiding this comment

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

This was previously released with 1.1

CHANGELOG.md Outdated
* Get start-docs working on linux ([#8486](https://github.com/mapbox/mapbox-gl-js/pull/8486))
* Add *-sort-key layout property for circle, fill, line ([#8467](https://github.com/mapbox/mapbox-gl-js/pull/8467))
* Include Hiragana and Katakana code blocks for local glyph generation ([#8365](https://github.com/mapbox/mapbox-gl-js/pull/8365))
* Use browser `Cache` interface to cache tiles ([#8363](https://github.com/mapbox/mapbox-gl-js/pull/8363))
Copy link
Contributor

Choose a reason for hiding this comment

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

This was previously released with 1.1

CHANGELOG.md Outdated
* Include Hiragana and Katakana code blocks for local glyph generation ([#8365](https://github.com/mapbox/mapbox-gl-js/pull/8365))
* Use browser `Cache` interface to cache tiles ([#8363](https://github.com/mapbox/mapbox-gl-js/pull/8363))
* Make requirements for text offset props more precise ([#8418](https://github.com/mapbox/mapbox-gl-js/pull/8418))
* Refactor normalize and canonicalize URL methods ([#8456](https://github.com/mapbox/mapbox-gl-js/pull/8456))
Copy link
Contributor

Choose a reason for hiding this comment

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

Internal refactors don't need a changelog entry

CHANGELOG.md Outdated
* Opaque pass layers now render properly over heatmap and fill-extrusion layers ([#8440](https://github.com/mapbox/mapbox-gl-js/pull/8440))
* Turn off 'move' event listeners when removing a marker ([#8465](https://github.com/mapbox/mapbox-gl-js/pull/8465))
* Favor previous anchor only when still in the `text-variable-anchor` options ([#8473](https://github.com/mapbox/mapbox-gl-js/pull/8473))
* Fix vector tile line rendering regression `bug` ([#8477](https://github.com/mapbox/mapbox-gl-js/issues/8477), fixed by [#8479](https://github.com/mapbox/mapbox-gl-js/pull/8479))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: bug

CHANGELOG.md Outdated
* Turn off 'move' event listeners when removing a marker ([#8465](https://github.com/mapbox/mapbox-gl-js/pull/8465))
* Favor previous anchor only when still in the `text-variable-anchor` options ([#8473](https://github.com/mapbox/mapbox-gl-js/pull/8473))
* Fix vector tile line rendering regression `bug` ([#8477](https://github.com/mapbox/mapbox-gl-js/issues/8477), fixed by [#8479](https://github.com/mapbox/mapbox-gl-js/pull/8479))
* Fix malformed URLs generated from normalizeTileURL and makeAPIURL (h/t @sumarlidason) ([#8466](https://github.com/mapbox/mapbox-gl-js/pull/8466))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: sumarlidason is a mapbox-contributor

CHANGELOG.md Outdated
* Fix Response construction in Edge ([#8392](https://github.com/mapbox/mapbox-gl-js/pull/8392))
* Fix class toggling on control for IE ([#8495](https://github.com/mapbox/mapbox-gl-js/pull/8495)) (h/t [cs09g](https://github.com/cs09g))
* Fix background rotation hovering ([#8367](https://github.com/mapbox/mapbox-gl-js/pull/8367)) (h/t [GuillaumeGomez](https://github.com/GuillaumeGomez))
* Fix broken map-tiles example ([#8482](https://github.com/mapbox/mapbox-gl-js/pull/8482))
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't need Changelog entry - fix was pushed directly to publisher-production.

CHANGELOG.md Outdated
* Fix class toggling on control for IE ([#8495](https://github.com/mapbox/mapbox-gl-js/pull/8495)) (h/t [cs09g](https://github.com/cs09g))
* Fix background rotation hovering ([#8367](https://github.com/mapbox/mapbox-gl-js/pull/8367)) (h/t [GuillaumeGomez](https://github.com/GuillaumeGomez))
* Fix broken map-tiles example ([#8482](https://github.com/mapbox/mapbox-gl-js/pull/8482))
* Fix unbounded memory growth in IE ([#8490](https://github.com/mapbox/mapbox-gl-js/pull/8490))
Copy link
Contributor

Choose a reason for hiding this comment

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

Already in v1.1.1

CHANGELOG.md Outdated
* Fix background rotation hovering ([#8367](https://github.com/mapbox/mapbox-gl-js/pull/8367)) (h/t [GuillaumeGomez](https://github.com/GuillaumeGomez))
* Fix broken map-tiles example ([#8482](https://github.com/mapbox/mapbox-gl-js/pull/8482))
* Fix unbounded memory growth in IE ([#8490](https://github.com/mapbox/mapbox-gl-js/pull/8490))
* Get map to load in Edge over http ([#8411](https://github.com/mapbox/mapbox-gl-js/issues/8411), fixed by [#8415](https://github.com/mapbox/mapbox-gl-js/pull/8415))
Copy link
Contributor

Choose a reason for hiding this comment

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

ALso in v1.1.1

CHANGELOG.md Outdated
* Get map to load in Edge over http ([#8411](https://github.com/mapbox/mapbox-gl-js/issues/8411), fixed by [#8415](https://github.com/mapbox/mapbox-gl-js/pull/8415))
* Fix error in certain click events on markers ([#8462](https://github.com/mapbox/mapbox-gl-js/pull/8462)) (h/t [@msbarry](https://github.com/msbarry))
* Fix SDK support spec section for variable label placement ([#8384](https://github.com/mapbox/mapbox-gl-js/pull/8384)) (h/t [@pozdnyakov](https://github.com/pozdnyakov))
* Fix regression in tile load times ([#8431](https://github.com/mapbox/mapbox-gl-js/issues/8431), fixed by [#8434](https://github.com/mapbox/mapbox-gl-js/pull/8434))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could use a better description

## 13.7.1


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: unneccessary newline

CHANGELOG.md Outdated
* Get start-docs working on linux ([#8486](https://github.com/mapbox/mapbox-gl-js/pull/8486))
* Add *-sort-key layout property for circle, fill, line ([#8467](https://github.com/mapbox/mapbox-gl-js/pull/8467))
* Make requirements for text offset props more precise ([#8418](https://github.com/mapbox/mapbox-gl-js/pull/8418))
* Get Codegen to remove extraneous newlines. ([6fdbdeb](https://github.com/mapbox/mapbox-gl-js/commit/6fdbdebd25de292ab1d42e4e6beff05fbf696d2e))
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be in changelog (no user-facing changes) and this is addressing an earlier PR in the same release.

CHANGELOG.md Outdated
* Add *-sort-key layout property for circle, fill, line ([#8467](https://github.com/mapbox/mapbox-gl-js/pull/8467))
* Make requirements for text offset props more precise ([#8418](https://github.com/mapbox/mapbox-gl-js/pull/8418))
* Get Codegen to remove extraneous newlines. ([6fdbdeb](https://github.com/mapbox/mapbox-gl-js/commit/6fdbdebd25de292ab1d42e4e6beff05fbf696d2e))
* Update integration dependencies ([#8354](https://github.com/mapbox/mapbox-gl-js/pull/8354))
Copy link
Member

Choose a reason for hiding this comment

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

This is only relevant for core contributors.

CHANGELOG.md Outdated
* Add map-specific access tokens ([#8364](https://github.com/mapbox/mapbox-gl-js/pull/8364))
* Explicit source options now take precedence over TileJSON ([#8232](https://github.com/mapbox/mapbox-gl-js/pull/8232)) (h/t [jingsam](https://github.com/jingsam))
* Accommodate prefers-reduced-motion settings in browser ([#8494](https://github.com/mapbox/mapbox-gl-js/pull/8494))
* Tilt compass as the map pitches ([#8208](https://github.com/mapbox/mapbox-gl-js/issues/8208), fixed by [#8296](https://github.com/mapbox/mapbox-gl-js/pull/8296)) (h/t [pakastin](https://github.com/pakastin))
Copy link
Member

Choose a reason for hiding this comment

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

Compass isn't tilted by default so I would clarify that as "Add Map visualizePitch option that ...".

CHANGELOG.md Outdated
* Add sdk support spec section for text-radial-offset ([#8401](https://github.com/mapbox/mapbox-gl-js/pull/8401))

## Bug fixes
* Opaque pass layers now render properly over heatmap and fill-extrusion layers ([#8440](https://github.com/mapbox/mapbox-gl-js/pull/8440))
Copy link
Member

Choose a reason for hiding this comment

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

For consistency, let's keep items in the same form (i.e. "Fix rendering of ....")

CHANGELOG.md Outdated
* Opaque pass layers now render properly over heatmap and fill-extrusion layers ([#8440](https://github.com/mapbox/mapbox-gl-js/pull/8440))
* Turn off 'move' event listeners when removing a marker ([#8465](https://github.com/mapbox/mapbox-gl-js/pull/8465))
* Favor previous anchor only when still in the `text-variable-anchor` options ([#8473](https://github.com/mapbox/mapbox-gl-js/pull/8473))
* Fix vector tile line rendering regression ([#8477](https://github.com/mapbox/mapbox-gl-js/issues/8477), fixed by [#8479](https://github.com/mapbox/mapbox-gl-js/pull/8479))
Copy link
Member

Choose a reason for hiding this comment

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

That's too broad. The PR fixes artifacts on tiles where lines go past allowed extent, and also fixes warning about this not appearing.

CHANGELOG.md Outdated
* Turn off 'move' event listeners when removing a marker ([#8465](https://github.com/mapbox/mapbox-gl-js/pull/8465))
* Favor previous anchor only when still in the `text-variable-anchor` options ([#8473](https://github.com/mapbox/mapbox-gl-js/pull/8473))
* Fix vector tile line rendering regression ([#8477](https://github.com/mapbox/mapbox-gl-js/issues/8477), fixed by [#8479](https://github.com/mapbox/mapbox-gl-js/pull/8479))
* Fix malformed URLs generated from normalizeTileURL and makeAPIURL ([#8466](https://github.com/mapbox/mapbox-gl-js/pull/8466))
Copy link
Member

Choose a reason for hiding this comment

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

normalizeTileURL and makeAPIURL are internal functions that are not exposed publicly, so this needs to be rephrased to reflect what changes on the user side. E.g. "Fix malformed urls when using custom baseAPIURL of a certain form".

CHANGELOG.md Outdated
* Fix class toggling on navigation control for IE ([#8495](https://github.com/mapbox/mapbox-gl-js/pull/8495)) (h/t [cs09g](https://github.com/cs09g))
* Fix background rotation hovering on geolocate control ([#8367](https://github.com/mapbox/mapbox-gl-js/pull/8367)) (h/t [GuillaumeGomez](https://github.com/GuillaumeGomez))
* Fix error in click events on markers where startPos is not defined ([#8462](https://github.com/mapbox/mapbox-gl-js/pull/8462)) (h/t [@msbarry](https://github.com/msbarry))
* Fix SDK support spec section for variable label placement ([#8384](https://github.com/mapbox/mapbox-gl-js/pull/8384)) (h/t [@pozdnyakov](https://github.com/pozdnyakov))
Copy link
Member

Choose a reason for hiding this comment

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

Both "SDK support spec section ..." items aren't clear for GL JS users. How they really experience this change is updated version support tables in docs, which I'm not sure we should surface in this changelog.

CHANGELOG.md Outdated
## 1.2.0

## Features and improvements
* Add *-sort-key layout property for circle, fill, line ([#8467](https://github.com/mapbox/mapbox-gl-js/pull/8467))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we add a few words about what this style-spec property enables? e.g.

Add *-sort-key layout property for circle, fill, line layers to enable sorting features within a single layer

Copy link
Contributor

Choose a reason for hiding this comment

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

I would also prefer that we use the `` around style-spec property names; I'm not sure if this is a convention we've been following in practice. so that this entry would say *-sort-key

CHANGELOG.md Outdated

## Features and improvements
* Add *-sort-key layout property for circle, fill, line ([#8467](https://github.com/mapbox/mapbox-gl-js/pull/8467))
* Add map-specific access tokens ([#8364](https://github.com/mapbox/mapbox-gl-js/pull/8364))
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do? If it's not that important, it should be lower in the changelog; if it is that important, its importance and impact should be clearer in the changelog entry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It enables users to add maps with access tokens specific to the map, but I'm struggling to make that more clear beyond switching the words around 🤔as for importance, there aren't many other big feature enhancements in this release overall, hence the prioritization

CHANGELOG.md Outdated
## Features and improvements
* Add *-sort-key layout property for circle, fill, line ([#8467](https://github.com/mapbox/mapbox-gl-js/pull/8467))
* Add map-specific access tokens ([#8364](https://github.com/mapbox/mapbox-gl-js/pull/8364))
* Make requirements for text offset props more precise ([#8418](https://github.com/mapbox/mapbox-gl-js/pull/8418))
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we limit non-standard abbreviations ("props") and try to be more precise about style-spec property names? e.g. for this entry:

Clarify requirements and restrictions for text-variable-anchor, text-radial-offset, text-offset and text-anchor properties in documentation

CHANGELOG.md Outdated
* Make requirements for text offset props more precise ([#8418](https://github.com/mapbox/mapbox-gl-js/pull/8418))
* Accommodate prefers-reduced-motion settings in browser ([#8494](https://github.com/mapbox/mapbox-gl-js/pull/8494))
* Add Map visualizePitch option that tilts the compass as the map pitches ([#8208](https://github.com/mapbox/mapbox-gl-js/issues/8208), fixed by [#8296](https://github.com/mapbox/mapbox-gl-js/pull/8296)) (h/t [pakastin](https://github.com/pakastin))
* Explicit source options now take precedence over TileJSON ([#8232](https://github.com/mapbox/mapbox-gl-js/pull/8232)) (h/t [jingsam](https://github.com/jingsam))
Copy link
Contributor

Choose a reason for hiding this comment

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

This entry doesn't meet GL-JS documentation requirements for starting with a verb. Could be rephrased as:

Allow explicit source options to take precedence over TileJSON

or

Make explicit source options take precedence over TileJSON

CHANGELOG.md Outdated

## Bug fixes
* Fix rendering of opaque pass layers over heatmap and fill-extrusion layers ([#8440](https://github.com/mapbox/mapbox-gl-js/pull/8440))
* Favor previous anchor only when still in the `text-variable-anchor` options ([#8473](https://github.com/mapbox/mapbox-gl-js/pull/8473))
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this PR name is clear as a changelog entry and would prefer:

Fix bug where previous anchors set in text-variable-anchor were still preferred after anchor options were reset

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the change emphasizes that previous positions would get prioritized only if they're also present in the updated style value, but i'll figure out a way to articulate that

@chloekraw
Copy link
Contributor

chloekraw commented Jul 22, 2019

@peterqliu, I did a pass through merged PRs for notable PRs that might be missing from the changelog. I think the only things are the caching fixes @ansis made:

I'd say

Fix unbounded memory growth caused by failure to cancel requests to the cache (#8472)

should definitely be included in the 1.2 changelog;

#8392 also looks significant enough to me for the changelog, since this regression was released in 1.1 and cherrypicked & fixed in 1.1.1. However, I noticed it also wasn't included in the 1.1.1 changelog, so I guess it could be clearer to exclude the fix from both changelogs.

Copy link
Contributor

@chloekraw chloekraw left a comment

Choose a reason for hiding this comment

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

LGTM % a few comments to consider

CHANGELOG.md Outdated
* Expose `convertFilter` API in the style specification ([#8493](https://github.com/mapbox/mapbox-gl-js/pull/8493)

## Bug fixes
* BREAKING: Fix changes to `text-variable-anchor`, such that previous anchor positions would take precedence only if they are present in the updated array (considered a bug fix, but is technically a breaking change from previous behavior) ([#8473](https://github.com/mapbox/mapbox-gl-js/pull/8473))
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, was this change made based on feedback from @mourner or @asheemmamoowala? If so, LGTM.

If not: specifying a breaking change like this is confusing to me as a user. I would prefer the following:

I prefer option 1 -- I don't think that the impact of this change is large or will break customers' maps in unexpected ways.

CHANGELOG.md Outdated
## 1.2.0

## Features and improvements
* Add `*-sort-key` layout property for circle, fill, and line layers, to sort the z-index of features within a single layer([#8467](https://github.com/mapbox/mapbox-gl-js/pull/8467))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how much this distinction matters, but *-sort-key impacts the draw order of features which is what makes it appear as though it's possible to sort features within a layer. Now that I'm reading this changelog entry with the addition of "z-index," it adds a level of precision that now makes the changelog entry feel less accurate to me, but I would defer to @ahk on this.

I'd probably prefer "...to control the draw order of features..." or the more general "to sort features within a single layer."

Copy link
Contributor

Choose a reason for hiding this comment

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

How about "Add *-sort-key layout property for circle, fill, and line layers. Allows specifying the order that features are drawn within a layer."

@peterqliu peterqliu merged commit d2c3a43 into release-picklejuice Jul 23, 2019
@peterqliu peterqliu deleted the 1.2.0.beta branch July 23, 2019 02:27
@@ -1,3 +1,26 @@
## 1.2.0

## Features and improvements
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: subtopics of the version should be H3 aka ### . Could use an edit to v1.1.1 as well

Copy link
Contributor

Choose a reason for hiding this comment

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

@mourner
Copy link
Member

mourner commented Jul 23, 2019

@chloekraw @peterqliu since all the caching fixes were included in v1.1.1, why would we include them in v1.2.0? Changelog should reflect changes since the previous version, not the previous minor version, so duplicating these entries would be confusing.

@chloekraw
Copy link
Contributor

@mourner ahh ok, that makes sense. I'm indifferent as to whether we let this one be or we open a new pr to fix it. It seems small enough that it's probably not a huge deal either way.

I'm accustomed to publishing a retroactive patch release (e.g. 5.1.1) after the latest minor release (5.2) has been published, in which case, a patch's changelog would still contain bug fix entries from the latest minor release.

@mourner
Copy link
Member

mourner commented Jul 23, 2019

I'm accustomed to publishing a retroactive patch release (e.g. 5.1.1) after the latest minor release (5.2) has been published, in which case, a patch's changelog would still contain bug fix entries from the latest minor release.

@chloekraw that's a reverse situation — a retroactive v5.1.1 release would show changes vs the previous release relative to it (v5.1.0). v1.2.0 on the other hand is not retroactive — it contains all the fixes v1.1.1 has.

@chloekraw
Copy link
Contributor

chloekraw commented Jul 23, 2019

yep! I'm agreeing with you; I was sharing that I missed how this convention (to only show changes since the previous release) impacts changelogs when there is a patch because I'm accustomed to the reverse situation.

@chloekraw
Copy link
Contributor

I'm all for fixing this and the heading in a new pr if that's still easy to do at this stage of the release.

peterqliu added a commit that referenced this pull request Jul 29, 2019
mourner added a commit that referenced this pull request Jul 30, 2019
* origin/publisher-production:
  update version to v1.2.0 (#8562)
  Fix usage of String.includes which is es6 but not transformed by buble (#8565) (#8573)
  [docs] fix missing location prop to feedback component (#8554)
  Fix typo
  [docs] update dr-ui (#8548)
  [docs] fix errors on IE11 caused by dr-ui dependencies (#8544)
  updates directions plugin version (#8536)
  add dds sdk support
  squash (#8529)
  [docs] adds feedback component (#8507)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants