Multi-file running changelog strategy #6740

SevaZhukov · 2022-12-13T11:51:19Z

Our current running changelog strategy is a reason for merge conflicts in every 1-3 PR. The new Multi-file running changelog strategy will solve this problem. How will it work you can read here.

@VysotskiVadim already tried to run Multi-file running changelog strategy in #5354 but he was faced with the need for maintaining a lot of code on CI for assembling changes. Now we can delegate assembling to the release train app.

create README.md about the new strategy
move unreleased features to json files
refactor CI scripts with verification changelog
fix tests of these scripts
refactor the train's changelog steps

dzinad · 2022-12-13T15:00:18Z

changelog/unreleased/6540.json

+[
+  {
+    "type": "Bug fixes and improvements",
+    "title": "Fixed approaches list update in `RouteOptionsUpdater`(uses for reroute). It was putting to the origin approach corresponding approach from legacy approach list."


Will the PR links be added automatically?

dzinad · 2022-12-13T15:08:05Z

changelog/unreleased/6540.json

@@ -0,0 +1,6 @@
+[
+  {
+    "type": "Bug fixes and improvements",


I see how we can easily make a typo here and get a weird header. Can we make it fool-proof? For example:
We only have a finite number of headers. Features, Bug fixes and improvements, Known issues.
Can we create directories for them? And then what's left is place a change entry as a plain string.
So for this example it would be:

find directory "changelog/unreleased/Bug fixes and improvements"

Create a file "6540.json"

The file contents:

[ "Fixed approaches list update in `RouteOptionsUpdater`(uses for reroute). It was putting to the origin approach corresponding approach from legacy approach list." ]

If there are multiple entries, we just add a second array item.

What I want to do here is to not use any json keys. Because it creates room for errors as well as typing "Bug fixes and improvements" every time.
I think an array of string would be sufficient, WDYT?

If the only field that left in file is title, maybe we can replace json with md?

I think it's a good point. In our case we have only 2 main parts: features and bugfixes, we can move type to path and make changelog file as md file

dzinad · 2022-12-13T15:11:34Z

changelog/README.md

@@ -0,0 +1,59 @@
+# Multi-file running changelog


We should definitely adapt changelog verification script... But first we have to agree on a format. I've left some suggestions.
I think we will have to check then:

A file (at least one) has been added to "changelog/unreleased";

The file is called PR_NUMBER.json;

It's a valid json array of strings;

Maybe something else, I'll have to look at the current checks.

tomaszrybakiewicz · 2022-12-13T15:14:39Z

Few things to keep in mind. Changelog entries may have:

code snippets

fun foo(bar: String) = "lorem ipsum dolor sit ammet..."

foldable sections (<details><summary></summary></details>)
Open for cake
Cake is a lie!!!
bold, italic and inline code text style
emojis ⚠️
images

changelog/example.json

kmadsen · 2022-12-14T01:32:13Z

Spent some time thinking/discussing and am sharing that I'm in favor. Great idea, and I like how it is something that GitLab had an issue with and came to a similar conclusion. They liked the solution enough to write a blog post https://about.gitlab.com/blog/2018/07/03/solving-gitlabs-changelog-conflict-crisis/

I have some minor opinions on the details, but overall I'm in favor. Cleaner solution than what we're doing now, the issue will be fixed, and it can be expanded for more improvements 👍

kmadsen · 2022-12-14T01:37:08Z

changelog/README.md

+- Mapbox Java `v6.8.0` ([release notes](https://github.com/mapbox/mapbox-java/releases/tag/v6.8.0))
+- Mapbox Android Core `v5.0.2` ([release notes](https://github.com/mapbox/mapbox-events-android/releases/tag/core-5.0.2))
+```
+* write the changelog to the `CHANGELOG.md` file


capturing @tomaszrybakiewicz comment: Instead of generating CHANGELOG during release, do this on PR merge.

I'm also +1'ing that. It is nice to see a running changelog, instead of needing to dig through each new file created.

Considering that may re-introduce the merge conflict issue, maybe it could be part of a script where we can say sh scripts/show-unreleased-changelog.sh? Preferably, there is a way to make it so we can have a running changelog

an idea is to have a file that represent the latest. every time a new pull request is merged the other one is deleted.

pr 01 is merged 01-changelog.md is created pr 02 is merged 01-changelog.md is deleted 02-changelog.md is created

not the best solution considering that the unreleased changelog link would have a short life https://github.com/mapbox/mapbox-navigation-android/blob/main/unreleased/01-changelog.md

Considering that may re-introduce the merge conflict issue,...

After merging a PR, the automation would append a new entry to the latest CHANGELOG.md file on the main branch. How would that create a conflict?

(main) * PR 1 merged * CHANGELOG.md update: PR 1 entry * PR 2 merged * CHANGELOG.md update: PR 2 entry

Do we want to keep current not released version of running changelog publicly available?
If no, maybe we can generate current changelog by request inside the release train app?

After merging a PR,

it means that changelog must be merged separately, in another commit, out of PR merge. It seems risky: make it in 1 step (aka transaction) safer, than drop to a few

also, this way we will have 2x commits in the branch (1 is changed + 1 changelog)

Do we really need unreleased changelog in one file before release?

it means that changelog must be merged separately, in another commit, out of PR merge. It seems risky: make it in 1 step (aka transaction) safer, than drop to a few

@RingerJK It's as risky as Release Train or CI release workflows making additional commits.

also, this way we will have 2x commits in the branch (1 is changed + 1 changelog)

And why is that a problem?

I think we can add one more job for the main branch to generate a running changelog file. This job will be executed after every merge

VysotskiVadim · 2022-12-14T09:00:37Z

changelog/README.md

+[
+  {
+    "type": "Features",
+    "title": "Description of changes"


is title a string that contains markdown?

VysotskiVadim · 2022-12-14T09:07:19Z

changelog/README.md

+
+To avoid merge conflicts in the CHANGELOG.md file we accepted the multi-file running changelog strategy.
+
+To follow this strategy you should create a `changelog/unreleased/${PR_NUMBER}.json` file for every PR like:


we usually add many lines during bump of NN and common dependencies. Does it mean that we will have to add as many files as many line we want to add during update of NN?

yeah, if we move features/bugfixes type to the path we can add files with additional info like this in the unreleased directory and structure will be like:

changelog --unreleased ----features ------feature_1.md ----bugfixes ------bugfix_1.md ----other_change_1.md ----other_change_2.md

VysotskiVadim · 2022-12-14T09:43:17Z

Should we let to insert sections like this or it should be done manually during release?

RingerJK · 2022-12-15T10:38:21Z

changelog/README.md

+
+To avoid merge conflicts in the CHANGELOG.md file we accepted the multi-file running changelog strategy.
+
+To follow this strategy you should create a `changelog/unreleased/${PR_NUMBER}.json` file for every PR like:


for that approach, we have to add a changelog in another commit (first create PR, get the PR number, and put the changelog and PR number into the branch).

Can it be modified so that we don't add the PR number manually? Can it be added under the hood when PR has been created?

for that approach, we have to add a changelog in another commit

I like to use the -f option but yes, I agree that it can get annoying.

I understand that it's not comfortable to write a changelog only after creating PR. I can propose creating an action that will rename the changelog file to ${PR_NUMBER}.md:

You create a PR (with changelog file feature_1.md)

The action executes and rename the changelog file to ${PR_NUMBER}.md
Action will be executed only on open/reopen/redy for review moments
WDYT

lgtm 👍

how does it works for release branches where we mostly cherry-pick changes (sometimes already released)? ref #6740 (comment)

oh it's a good moment, I think we can create a PR to the release branch first. Then, the action will rename the changelog file. Then, we can cherry-pick commits (changes + renaming commit) to the main branch

oh it's a good moment, I think we can create a PR to the release branch first. Then, the action will rename the changelog file. Then, we can cherry-pick commits (changes + renaming commit) to the main branch

Typically we merge into main branch, and then cherry-pick into the release branch(es). Maybe minor detail but I don't think we want it to require release commits first.

RingerJK · 2022-12-15T10:59:48Z

@SevaZhukov would be any corner cases for release branches? Could you mention if any in readme

SevaZhukov · 2022-12-15T15:17:41Z

Should we let to insert sections like this or it should be done manually during release?

Good point, I have added this section

codecov · 2022-12-15T16:03:20Z

Codecov Report

Merging #6740 (f8d89a7) into main (14571b9) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #6740   +/-   ##
=========================================
  Coverage     72.51%   72.51%           
  Complexity     5551     5551           
=========================================
  Files           779      779           
  Lines         30072    30072           
  Branches       3548     3548           
=========================================
  Hits          21806    21806           
  Misses         6841     6841           
  Partials       1425     1425

dzinad · 2022-12-15T16:22:19Z

changelog/README.md

+```
+
+You can choose any name for your changelog files because the GitHub action will rename files in
+`changelog/unreleased/features` and `changelog/unreleased/bugfixes` directories to `${PR_NUMBER}.md` when you open a PR.


What about the issues directory and just changelog/unreleased?

we need PR's links only for bugfixes and features

dzinad · 2022-12-15T16:23:36Z

changelog/README.md

+```
+
+* write the changelog to the `CHANGELOG.md` file
+* delete all files from `changelog/unreleased`


Just make sure patch releases won't erase all main changelog entries...

Releases since rc.1 will erase changelog/unreleased dir only in release branches

dzinad · 2022-12-15T16:23:52Z

changelog/unreleased/bugfixes/6540.md

@@ -0,0 +1 @@
+Fixed approaches list update in `RouteOptionsUpdater`(uses for reroute). It was putting to the origin approach corresponding approach from legacy approach list.


Shouldn't there be a - in the beginning?

I think the script will add - during assembling

It's not a very good idea. If the entry is multi-line, how will the script understand where to add the -?

if you have only one feture/bugfix you can don't use - before, if you have more you should

dzinad · 2022-12-15T16:24:01Z

changelog/unreleased/bugfixes/6695.md

@@ -0,0 +1 @@
+Updated the `MapboxRestAreaApi` logic to load a SAPA map only if the upcoming rest stop is at the current step of the route leg.


Shouldn't there be a - in the beginning?

dzinad · 2022-12-15T16:24:17Z

changelog/unreleased/issues/example-known-issues.md

@@ -0,0 +1 @@
+It is an example of known issues


Shouldn't there be a - in the beginning?

dzinad · 2022-12-15T16:25:03Z

scripts/validate_changelog_utils.py

 diff_file_start_regex = "^([\s]*)\@\@(.*)\@\@"
 pr_link_regex = "\[#\d+]\(https:\/\/github\.com\/mapbox\/mapbox-navigation-android\/pull\/\d+\)"

+


That's not what I meant when saying we should adapt the checks...

I didn't do anything with it))

But there are changes in the diff...

only autoformating

dzinad · 2022-12-15T16:27:08Z

changelog/README.md

+- Description of changes in md format also
+```
+
+You can choose any name for your changelog files because the GitHub action will rename files in


I think it's worth mentioning that you can't create 2 files in 1 directory in 1 PR. I don't think we someone would want that (I don't see any cases for that) but still we should note that.

but I mentioned it here

It doesn't say you can't create mmultiple files.

SevaZhukov · 2022-12-20T12:19:20Z

hey team, I added two GitHub actions

Rename changelog files. It executes every update and looking for and renaming new changelog files in features and bugfixes dirs from any-feature-name.md to ${PR_NUMBER}.md. After it pushes to the current branch.
Assemble changelog. It executes only on the main and release branches. This action compile changelog from features, bugfixes and issues dirs like:

#### Features
- Test changes to check renaming
- Introduced `MapboxSpeedInfoApi` and `MapboxSpeedInfoView`. The combination of API and View can be used to render posted and current speed limit at user's current location.
- :warning: Deprecated `MapboxSpeedLimitApi` and `MapboxSpeedLimitView`.

#### Bug fixes and improvements
- Updated the `MapboxRestAreaApi` logic to load a SAPA map only if the upcoming rest stop is at the current step of the route leg.
- Fixed approaches list update in `RouteOptionsUpdater`(uses for reroute). It was putting to the origin approach corresponding approach from legacy approach list.

#### Known issues :warning:
- It is an example of known issues

After it commits and pushes to the current branch. Commit name contains [skip actions] label to skip this commit for the actions.

@tomaszrybakiewicz @kmadsen @VysotskiVadim @RingerJK @dzinad

dzinad · 2022-12-20T13:20:14Z

changelog/README.md

+
+You can use anything that allow .md format in changelog files.
+
+If you have implemented some features or bugfixes you should describe all of them:


some -> several/multiple

dzinad · 2022-12-20T13:22:55Z

changelog/README.md

+You can choose any name for your changelog files because the GitHub action will rename files in
+`changelog/unreleased/features` and `changelog/unreleased/bugfixes` directories to `${PR_NUMBER}.md` when you open a PR.
+
+Every release the release train app will:


It's not only on every release, right? It will happen after every merge? Can you actualize README? Because that's all I'm reviewing actually: this file and the examples. :)

dzinad · 2022-12-20T13:23:53Z

changelog/unreleased/CHANGELOG.md

@@ -0,0 +1,11 @@
+#### Features
+- Test changes to check renaming


What about the PR links? They should be here because we've lost all the info about where this entry came from at this point, right?

I added links to assembled changelog, thanks

dzinad · 2022-12-20T14:32:00Z

changelog/README.md

+Some other changes
+```
+
+* write the changelog to the `changelog/unreleased/CHANGELOG.md` file


What about the release process? Shouldn't we move the changelog from unreleased directory to the project root and delete the unreleased one?

Added more information

dzinad · 2022-12-20T14:32:22Z

changelog/README.md

+You can choose any name for your changelog files because the GitHub action will rename files in
+`changelog/unreleased/features` and `changelog/unreleased/bugfixes` directories to `${PR_NUMBER}.md` when you open a PR.
+
+Every push to the main or release branch Assemble changelog GitHub action will be executed:


The temporary files will be deleted, right?

no, let's keep they before the release

dzinad · 2022-12-21T11:08:02Z

The docs and the examples look good. And I believe you know what you're doing with the scripts. :)

kmadsen · 2022-12-23T20:10:14Z

changelog/README.md

+- Mapbox Android Core `v5.0.2` ([release notes](https://github.com/mapbox/mapbox-events-android/releases/tag/core-5.0.2))
+```
+* add compiled changelog to `CHANGELOG.md` file
+* delete all files in `changelog/unreleased` dir


@SevaZhukov will this system work with the android auto changelog https://github.com/mapbox/mapbox-navigation-android/blob/main/libnavui-androidauto/CHANGELOG.md

All good to consider it out of scope but it would be nice if it supported multiple release channels

VysotskiVadim · 2023-01-03T09:27:22Z

changelog/README.md

+You can choose any name for your changelog files because the GitHub action will rename files in
+`changelog/unreleased/features` and `changelog/unreleased/bugfixes` directories to `${PR_NUMBER}.md` when you open a PR.
+
+Every push to the main or release branch Assemble changelog GitHub action will be executed:


Will those commit trigger metrics run? For now it seems so, because each PR merge produces 2 commits in the main branch.

[skip ci] tag allows skipping CircleCI jobs and GitHub Actions

VysotskiVadim · 2023-01-03T09:41:50Z

scripts/changelog/assemble_changelog.py

+import git
+
+
+def get_changes(path):


What is the testing strategy for the changelog logic? Do I understand correctly that after each change I need to manually run script locally simulating different scenarios that my change may affect? Do you think it makes sense to add auto tests, so that developers could enhance this logic without being afraid break a different scenario?

I think this question isn't a blocker for this particular PR, but it's worth to discuss 🙂

yeah, good point, let's add tests for this logic

RingerJK · 2023-01-03T14:08:00Z

changelog/README.md

+
+To follow this strategy you should create a `.md` file for every PR. Choose a directory:
+
+- `changelog/unreleased/features` for **Features** changes


IMO would be nice to have a script to add changelog like python add_changelog.py -f "Changelog string", where -f is feature (also should be other flags), "Changelog string" is the entry of changelog

(cherry picked from commit b329337)

dzinad reviewed Dec 13, 2022

View reviewed changes

kmadsen reviewed Dec 13, 2022

View reviewed changes

changelog/example.json Outdated Show resolved Hide resolved

kmadsen reviewed Dec 14, 2022

View reviewed changes

VysotskiVadim reviewed Dec 14, 2022

View reviewed changes

RingerJK reviewed Dec 15, 2022

View reviewed changes

dzinad reviewed Dec 15, 2022

View reviewed changes

SevaZhukov force-pushed the NAVSDK-646 branch from 8245f2b to 9d325a2 Compare December 20, 2022 10:35

dzinad reviewed Dec 20, 2022

View reviewed changes

SevaZhukov force-pushed the NAVSDK-646 branch from 54394ac to 11ae8c3 Compare December 21, 2022 09:37

dzinad approved these changes Dec 21, 2022

View reviewed changes

SevaZhukov marked this pull request as ready for review December 21, 2022 14:04

github-actions bot requested review from Guardiola31337 and korshaknn December 21, 2022 14:05

SevaZhukov force-pushed the NAVSDK-646 branch from 17e22ac to 17bda6f Compare December 21, 2022 14:22

kmadsen reviewed Dec 23, 2022

View reviewed changes

SevaZhukov force-pushed the NAVSDK-646 branch from bbd757f to 722c121 Compare January 3, 2023 08:22

VysotskiVadim reviewed Jan 3, 2023

View reviewed changes

RingerJK reviewed Jan 3, 2023

View reviewed changes

RingerJK approved these changes Jan 3, 2023

View reviewed changes

sevastian.zhukov and others added 26 commits January 4, 2023 19:24

test assemble changelog

87cbc1b

test assemble changelog

c83968d

test assemble changelog

79941ad

test assemble changelog

a12a9c8

fix

a8f69b4

Assemble changelog file [skip actions]

cc27e5a

fix conditions for assemble changelog action

5585d85

add PR links when assemble changelog

9e48ed8

update doc

8eb59dd

fix other and issues parts

daf3a90

information about release

abed7d4

information about release

1dcc54b

add pr links only for lines starts with '- '

4357b1d

update validation scripts

a9fe948

remove not relevant tests

fb8fa38

test formatting and fixes

b8ae890

fix tests

bf79b11

actualization changelog

ff7a3dd

fix rename changelog script

64cd1d8

android auto running changelog strategy

8966c79

fix new line symbol

4b65a30

actualize changelogs

16d5cd6

update doc for android auto

b084cfa

fix skip tag

ffb7291

add changelog file script

253915e

add changelog files

f8d89a7

SevaZhukov force-pushed the NAVSDK-646 branch from a1a5017 to f8d89a7 Compare January 4, 2023 12:35

SevaZhukov merged commit b329337 into main Jan 4, 2023

SevaZhukov deleted the NAVSDK-646 branch January 4, 2023 13:48

SevaZhukov pushed a commit that referenced this pull request Jan 10, 2023

Multi-file running changelog strategy (#6740)

f6b0874

(cherry picked from commit b329337)


		To avoid merge conflicts in the CHANGELOG.md file we accepted the multi-file running changelog strategy.

		To follow this strategy you should create a `changelog/unreleased/${PR_NUMBER}.json` file for every PR like:

		@@ -0,0 +1 @@
		Fixed approaches list update in `RouteOptionsUpdater`(uses for reroute). It was putting to the origin approach corresponding approach from legacy approach list. No newline at end of file

		@@ -0,0 +1 @@
		Updated the `MapboxRestAreaApi` logic to load a SAPA map only if the upcoming rest stop is at the current step of the route leg. No newline at end of file

		@@ -0,0 +1 @@
		It is an example of known issues No newline at end of file

		diff_file_start_regex = "^([\s])\@\@(.)\@\@"
		pr_link_regex = "\[#\d+]\(https:\/\/github\.com\/mapbox\/mapbox-navigation-android\/pull\/\d+\)"


		You can use anything that allow .md format in changelog files.

		If you have implemented some features or bugfixes you should describe all of them:

		@@ -0,0 +1,11 @@
		#### Features
		- Test changes to check renaming


		To follow this strategy you should create a `.md` file for every PR. Choose a directory:

		- `changelog/unreleased/features` for Features changes

		import git


		def get_changes(path):

Multi-file running changelog strategy #6740

Multi-file running changelog strategy #6740

Uh oh!

Conversation

SevaZhukov commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VysotskiVadim Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomaszrybakiewicz commented Dec 13, 2022

Uh oh!

Uh oh!

kmadsen commented Dec 14, 2022

Uh oh!

kmadsen Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmadsen Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VysotskiVadim Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VysotskiVadim commented Dec 14, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RingerJK commented Dec 15, 2022

Uh oh!

SevaZhukov commented Dec 15, 2022

Uh oh!

codecov bot commented Dec 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SevaZhukov commented Dec 13, 2022 •

edited

Loading

VysotskiVadim Dec 14, 2022 •

edited

Loading

kmadsen Dec 14, 2022 •

edited

Loading

kmadsen Dec 14, 2022 •

edited

Loading

VysotskiVadim Dec 14, 2022 •

edited

Loading

codecov bot commented Dec 15, 2022 •

edited

Loading