Assemble changelog only in PRs comments #6783

SevaZhukov · 2023-01-05T10:17:54Z

After further consideration, it has been determined that it is not possible to both assemble and push the changelog to protected branches. Using PRs to update the changelog would simply bring us back to the original issue. As a result, the script has been refactored to only add the assembled changelog to PRs comments.

dzinad · 2023-01-05T10:25:36Z

After further consideration, it has been determined that it is not possible to both assemble and push the changelog to protected branches.

Why? Is this because of some restrictions on the main branch?

dzinad · 2023-01-05T10:28:37Z

Can we print the changelog automatically in the PR? Like we do with codecov? Because I seriously doubt that someone will manually run the script.

codecov · 2023-01-05T10:50:10Z

Codecov Report

Merging #6783 (aef3ed1) into main (8322e43) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main    #6783   +/-   ##
=========================================
  Coverage     72.50%   72.50%           
  Complexity     5550     5550           
=========================================
  Files           779      779           
  Lines         30067    30067           
  Branches       3548     3548           
=========================================
  Hits          21801    21801           
  Misses         6841     6841           
  Partials       1425     1425

VysotskiVadim · 2023-01-05T13:30:43Z

release train will assemble the changelog automatically and open and merge the PR, right? Or release duty will have to do it manually?

VysotskiVadim · 2023-01-05T13:32:48Z

BTW, what is the flow now. I add a single changelog entry and merge pr without updating CHANGELOG.md. During release we assemble changelog changelog from entries and remove them. is my understanding correct?

VysotskiVadim · 2023-01-05T13:34:15Z

After further consideration, it has been determined that it is not possible to both assemble and push the changelog to protected branches.

Why? Is this because of some restrictions on the main branch?

yes, it's a protected branch. I remember that I had similar issue automating something.

SevaZhukov · 2023-01-05T13:57:11Z

release train will assemble the changelog automatically and open and merge the PR, right? Or release duty will have to do it manually?

the release train will do it automatically

SevaZhukov · 2023-01-05T14:00:54Z

BTW, what is the flow now. I add a single changelog entry and merge pr without updating CHANGELOG.md. During release we assemble changelog changelog from entries and remove them. is my understanding correct?

yes, I mentioned it here

VysotskiVadim · 2023-01-05T14:09:19Z

changelog/README.md


 Every release the release train app will:

-* get changelog from `changelog/unreleased/CHANGELOG.md` file
+* assemble the changelog by the script `python3 scripts/changelog/assemble_changelog.py`


If you add #!/usr/bin/env python3 to the top of the file, you can run script typing less 🙂

Suggested change

* assemble the changelog by the script `python3 scripts/changelog/assemble_changelog.py`

* assemble the changelog by the script `./scripts/changelog/assemble_changelog.py`

github-actions · 2023-01-05T15:23:05Z

Changelog

Features

Bug fixes and improvements

Each newly instantiated MapboxRouteArrowView class will initialize the layers with the provided options on the first render call. Previously this would only be done if the layers hadn't already been initialized. #6466

⚠️ Updated the NavigationView default navigation puck asset. #6678

Previous puck can be restored by injecting LocationPuck2D with the bearingImage set to com.mapbox.navigation.ui.maps.R.drawable.mapbox_navigation_puck_icon drawable:

navigationView.customizeViewStyles {
    locationPuckOptions = LocationPuckOptions.Builder(context)
        .defaultPuck(
            LocationPuck2D(
                bearingImage = ContextCompat.getDrawable(
                    context,
                    com.mapbox.navigation.ui.maps.R.drawable.mapbox_navigation_puck_icon,
                )
            )
        )
        .idlePuck(regularPuck(context))
        .build()
}

Fixed an issue with NavigationView that caused info panel to shrink in landscape mode with a full screen theme. #6780
Fixed standalone MapboxManeuverView appearance when the app also integrates Drop-In UI. #6774
Added guarantees that route progress with RouteProgress#currentState == OFF_ROUTE arrives earlier than NavigationRerouteController#reroute is called. #6764
Introduced NavigationViewListener.onSpeedInfoClicked that would be triggered when MapboxSpeedInfoView is clicked upon. #6770
Fixed a rare java.lang.NullPointerException: Attempt to read from field 'SpeechAnnouncement PlayCallback.announcement' on a null object reference crash in PlayCallback.getAnnouncement. #6760
Fixed an issue where the first voice instruction might have been played twice. #6766

Known issues ⚠️

Other changes

github-actions · 2023-01-05T15:23:06Z

Android Auto Changelog

Features

Bug fixes and improvements

This reverts commit aa6f7e8.

dzinad · 2023-01-05T15:51:40Z

scripts/changelog/assemble_changelog.py

 auto_bugfixes = get_changes('libnavui-androidauto/changelog/unreleased/bugfixes/')
 auto_features = get_changes('libnavui-androidauto/changelog/unreleased/features/')

-auto_changelog = '#### Features\n' + auto_features + '\n\n' + \
-            '#### Bug fixes and improvements\n' + auto_bugfixes
+auto_changelog = '# Android Auto Changelog\n' + \


Out of scope of this PR and it's your file but I would try to generalize something here for the default changelog and the android auto one. So that it's easier to:

Keep track of what changelogs are supported;

Make changes in all places at once.

I see a list of objects describing the changelog file instead of having variable like a, b, c, ..., auto_a, auto_b, auto_c, ... . The objects would have different paths, changelog file names, maybe something else.
Just a refactoring idea.

tomaszrybakiewicz · 2023-01-05T21:50:50Z

After further consideration, it has been determined that it is not possible to both assemble and push the changelog to protected branches.

Quoting GitHub blog post:

You can now create a custom role to bypass branch protections without having to grant the Admin role. Previously, to bypass branch protections you had to be an Admin which provides additional permissions that may not be needed. For tighter control of Admin permissions, you can now craft a custom role that has the Bypass branch protections permission, allowing just the right amount of access.

We could be a bit more strict with this repo permissions. Grant "navigation team" members the "Maintain" role, a couple leads Admin role and automation accounts Custom role with the Bypass branch protections permission.

@LukasPaczos @zugaldia @abhishek1508 @Guardiola31337 WDYT??

abhishek1508 · 2023-01-06T00:16:59Z

Do we need to assemble the changelog with every PR? If we are creating a separate *.md file with every PR, that hosts the change introduced, can we just not assemble all the changes by going through each *.md and adding them directly to root/CHANGELOG.md at the time of release? It probably made sense earlier, because we were directly modifying the root/CHANGELOG.md with every PR. This made it easy for any developers(internal/external) to look at the running changelog between releases. With the current change in main we are assembling the changes but inside root/changelog/unreleased/CHANGELOG.md. I am not sure how many developers would know to look for a running changelog there. With the changes in this branch, we are not even doing that and rather assembling the changes only in the PR. Thus there is no way for any external/internal developer to look at the running changelog between releases other than to go to that particular PR.

I am not in favor of adding the assembled changelog to PR comment as I am not sure what value am I getting out of it. This also pollutes the PR in case there are several discussions happening which by the way is most often the case based on code review. The changes in README.md mentions:

The comment will be updated with every change.

Does this mean the assembled changelog will be posted as PR comment every time a new commit is pushed or forced rebase is made?

SevaZhukov · 2023-01-06T09:00:28Z

We could be a bit more strict with this repo permissions. Grant "navigation team" members the "Maintain" role, a couple leads Admin role and automation accounts Custom role with the Bypass branch protections permission.

We can't give the bot permission Bypass branch protections anyway. Because the script uses the github-actions token, it's not a user.

SevaZhukov · 2023-01-06T09:57:44Z

Does this mean the assembled changelog will be posted as PR comment every time a new commit is pushed or forced rebase is made?

Yes, but it will create a comment if it doesn't exist or update if it exists.

github-actions · 2023-01-06T10:34:15Z

Changelog

Features

Bug fixes and improvements

Each newly instantiated MapboxRouteArrowView class will initialize the layers with the provided options on the first render call. Previously this would only be done if the layers hadn't already been initialized. #6466

⚠️ Updated the NavigationView default navigation puck asset. #6678

Previous puck can be restored by injecting LocationPuck2D with the bearingImage set to com.mapbox.navigation.ui.maps.R.drawable.mapbox_navigation_puck_icon drawable:

navigationView.customizeViewStyles {
    locationPuckOptions = LocationPuckOptions.Builder(context)
        .defaultPuck(
            LocationPuck2D(
                bearingImage = ContextCompat.getDrawable(
                    context,
                    com.mapbox.navigation.ui.maps.R.drawable.mapbox_navigation_puck_icon,
                )
            )
        )
        .idlePuck(regularPuck(context))
        .build()
}

Fixed an issue with NavigationView that caused info panel to shrink in landscape mode with a full screen theme. #6780
Fixed standalone MapboxManeuverView appearance when the app also integrates Drop-In UI. #6774
Added guarantees that route progress with RouteProgress#currentState == OFF_ROUTE arrives earlier than NavigationRerouteController#reroute is called. #6764
Introduced NavigationViewListener.onSpeedInfoClicked that would be triggered when MapboxSpeedInfoView is clicked upon. #6770
Fixed a rare java.lang.NullPointerException: Attempt to read from field 'SpeechAnnouncement PlayCallback.announcement' on a null object reference crash in PlayCallback.getAnnouncement. #6760
Fixed an issue where the first voice instruction might have been played twice. #6766

Known issues ⚠️

Other changes

Android Auto Changelog

Features

Bug fixes and improvements

SevaZhukov · 2023-01-06T10:38:07Z

I propose a compromise - a collapsible changelog comment. It doesn't take up a lot of space and it exists

abhishek1508 · 2023-01-06T19:25:10Z

I propose a compromise - a collapsible changelog comment. It doesn't take up a lot of space and it exists

This is a better solution in my opinion. Thanks for giving it a deeper thought.

However, I still don't understand the need for assembling the changelog in every PR and putting it as a PR comment. Can someone explain the benefit of it?

cc @mapbox/navigation-android

tomaszrybakiewicz · 2023-01-06T19:51:14Z

We could be a bit more strict with this repo permissions. Grant "navigation team" members the "Maintain" role, a couple leads Admin role and automation accounts Custom role with the Bypass branch protections permission.

We can't give the bot permission Bypass branch protections anyway. Because the script uses the github-actions token, it's not a user.

@SevaZhukov Can't we request a separate user, assign a custom role with Bypass branch protections and use its personal token for the workflow as suggested here? https://docs.github.com/en/actions/security-guides/automatic-token-authentication#granting-additional-permissions
Of course, we would still need to downgrade everyone to the "Maintainer" role and disable "Do not allow bypassing the above settings" branch protection to make it work.

SevaZhukov · 2023-01-09T09:27:22Z

Can't we request a separate user, assign a custom role with Bypass branch protections and use its personal token for the workflow as suggested here?

Sorry but no :( Because as I know we can't use and store permanent GitHub tokens. We can only get it by 1. mbx-ci (it doesn't work for this case because it's a public repo) 2. use default github-token for github-actions

dzinad · 2023-01-09T09:40:02Z

However, I still don't understand the need for assembling the changelog in every PR and putting it as a PR comment. Can someone explain the benefit of it?

Well, I would like to be able to see what the changelog will result in, especially in the beginning when I'm new to the flow and it is subject to change.

VysotskiVadim · 2023-01-09T09:54:46Z

However, I still don't understand the need for assembling the changelog in every PR and putting it as a PR comment. Can someone explain the benefit of it?

I think it's nice to see how new changelog entries affect the way whole changelog looks like. It may be useful if you write not a typical message, for instance with code examples or warnings about known issues.

(cherry picked from commit 671b157)

Assemble changelog only by the script executing

bea0230

SevaZhukov added the skip changelog Should not be added into version changelog. label Jan 5, 2023

github-actions bot requested review from dzinad and VysotskiVadim January 5, 2023 10:18

VysotskiVadim approved these changes Jan 5, 2023

View reviewed changes

SevaZhukov added 2 commits January 5, 2023 21:54

create and update changelog in pr comment

63ef969

update readme

978f125

SevaZhukov changed the title ~~Assemble changelog only by the script executing~~ Assemble changelog only in PRs comments Jan 5, 2023

SevaZhukov added 2 commits January 5, 2023 22:33

simple run script

aa6f7e8

Revert "simple run script"

3282087

This reverts commit aa6f7e8.

dzinad approved these changes Jan 5, 2023

View reviewed changes

collapsable changelog comment

aef3ed1

SevaZhukov merged commit 671b157 into main Jan 10, 2023

SevaZhukov deleted the szh-changelog-strategy-fixes branch January 10, 2023 09:05

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

Assemble changelog only in PRs comments (#6783)

dc30f35

(cherry picked from commit 671b157)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assemble changelog only in PRs comments #6783

Assemble changelog only in PRs comments #6783

SevaZhukov commented Jan 5, 2023 •

edited

Loading

dzinad commented Jan 5, 2023

dzinad commented Jan 5, 2023

codecov bot commented Jan 5, 2023 •

edited

Loading

VysotskiVadim commented Jan 5, 2023

VysotskiVadim commented Jan 5, 2023 •

edited

Loading

VysotskiVadim commented Jan 5, 2023 •

edited

Loading

SevaZhukov commented Jan 5, 2023

SevaZhukov commented Jan 5, 2023

VysotskiVadim Jan 5, 2023

github-actions bot commented Jan 5, 2023

github-actions bot commented Jan 5, 2023

dzinad Jan 5, 2023

tomaszrybakiewicz commented Jan 5, 2023 •

edited

Loading

abhishek1508 commented Jan 6, 2023 •

edited

Loading

SevaZhukov commented Jan 6, 2023

SevaZhukov commented Jan 6, 2023

github-actions bot commented Jan 6, 2023

Features

Bug fixes and improvements

Known issues ⚠️

Other changes

Features

Bug fixes and improvements

SevaZhukov commented Jan 6, 2023

abhishek1508 commented Jan 6, 2023

tomaszrybakiewicz commented Jan 6, 2023

SevaZhukov commented Jan 9, 2023

dzinad commented Jan 9, 2023

VysotskiVadim commented Jan 9, 2023

	* assemble the changelog by the script `python3 scripts/changelog/assemble_changelog.py`
	* assemble the changelog by the script `./scripts/changelog/assemble_changelog.py`

Assemble changelog only in PRs comments #6783

Assemble changelog only in PRs comments #6783

Conversation

SevaZhukov commented Jan 5, 2023 • edited Loading

dzinad commented Jan 5, 2023

dzinad commented Jan 5, 2023

codecov bot commented Jan 5, 2023 • edited Loading

Codecov Report

VysotskiVadim commented Jan 5, 2023

VysotskiVadim commented Jan 5, 2023 • edited Loading

VysotskiVadim commented Jan 5, 2023 • edited Loading

SevaZhukov commented Jan 5, 2023

SevaZhukov commented Jan 5, 2023

VysotskiVadim Jan 5, 2023

Choose a reason for hiding this comment

github-actions bot commented Jan 5, 2023

Changelog

Features

Bug fixes and improvements

Known issues ⚠️

Other changes

github-actions bot commented Jan 5, 2023

Android Auto Changelog

Features

Bug fixes and improvements

dzinad Jan 5, 2023

Choose a reason for hiding this comment

tomaszrybakiewicz commented Jan 5, 2023 • edited Loading

abhishek1508 commented Jan 6, 2023 • edited Loading

SevaZhukov commented Jan 6, 2023

SevaZhukov commented Jan 6, 2023

github-actions bot commented Jan 6, 2023

Features

Bug fixes and improvements

Known issues ⚠️

Other changes

Features

Bug fixes and improvements

SevaZhukov commented Jan 6, 2023

abhishek1508 commented Jan 6, 2023

tomaszrybakiewicz commented Jan 6, 2023

SevaZhukov commented Jan 9, 2023

dzinad commented Jan 9, 2023

VysotskiVadim commented Jan 9, 2023

SevaZhukov commented Jan 5, 2023 •

edited

Loading

codecov bot commented Jan 5, 2023 •

edited

Loading

VysotskiVadim commented Jan 5, 2023 •

edited

Loading

VysotskiVadim commented Jan 5, 2023 •

edited

Loading

tomaszrybakiewicz commented Jan 5, 2023 •

edited

Loading

abhishek1508 commented Jan 6, 2023 •

edited

Loading