-
Notifications
You must be signed in to change notification settings - Fork 321
Multi-file running changelog strategy #6740
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
Changes from all commits
2831885
e5f6e04
e2fbbe8
94b2ccf
780f542
3813aba
20386e7
8c28e15
2e9ace8
1f64183
6daece7
4699a72
8388caa
73cbfbd
78a521d
8c58767
69756b2
87ecfe2
e7286d6
87cbc1b
c83968d
79941ad
a12a9c8
a8f69b4
cc27e5a
5585d85
9e48ed8
8eb59dd
daf3a90
abed7d4
1dcc54b
4357b1d
a9fe948
fb8fa38
b8ae890
bf79b11
ff7a3dd
64cd1d8
8966c79
4b65a30
16d5cd6
b084cfa
ffb7291
253915e
f8d89a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| name: Assemble changelog | ||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| - release-v** | ||
| jobs: | ||
| process: | ||
| permissions: | ||
| pull-requests: write | ||
| contents: write | ||
| runs-on: ubuntu-20.04 | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| with: | ||
| ref: ${{github.head_ref}} | ||
|
|
||
| - name: setup python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.7.7' | ||
|
|
||
| - name: install python packages | ||
| run: | | ||
| python3 -m pip install requests GitPython | ||
|
|
||
| - name: execute py script | ||
| run: | | ||
| python3 scripts/changelog/assemble_changelog.py |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| name: Rename changelog files | ||
| on: | ||
| pull_request: | ||
| types: [ opened, reopened, ready_for_review, synchronize ] | ||
| jobs: | ||
| process: | ||
| permissions: | ||
| pull-requests: write | ||
| contents: write | ||
| runs-on: ubuntu-20.04 | ||
| env: | ||
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| steps: | ||
| - uses: actions/checkout@v3 | ||
| with: | ||
| ref: ${{github.head_ref}} | ||
|
|
||
| - name: setup python | ||
| uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.7.7' | ||
|
|
||
| - name: install python packages | ||
| run: | | ||
| python3 -m pip install requests GitPython | ||
|
|
||
| - name: execute py script | ||
| run: | | ||
| python3 scripts/changelog/rename_changelog_file.py |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| # Multi-file running changelog | ||
|
|
||
| To avoid merge conflicts in the CHANGELOG.md file we accepted the multi-file running changelog strategy. | ||
|
|
||
| *This strategy works for the libnavui-androidauto project too. It works in the `libnavui-androidauto/changelog` directory* | ||
|
|
||
| To follow this strategy you should create a `.md` file for every PR. Choose a directory: | ||
|
|
||
| - `changelog/unreleased/features` for **Features** changes | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO would be nice to have a script to add changelog like
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
| - `changelog/unreleased/bugfixes` for **Bug fixes and improvements** changes | ||
| - `changelog/unreleased/issues` for **Known issues :warning:** changes | ||
| - `changelog/unreleased/other` for other changes | ||
|
|
||
| Or you can use the helper script to do it. Just call `python3 scripts/changelog/add_changelog.py -f "I have added something special"` to create a changelog file. | ||
| Call `python3 scripts/changelog/add_changelog.py -h` to get more info. | ||
|
|
||
| You can use anything that allow .md format in changelog files. | ||
|
|
||
| If you have implemented several features or bugfixes you should describe all of them: | ||
|
|
||
| ``` | ||
| - Description of changes in md format | ||
| - Description of changes in md format also | ||
| ``` | ||
|
|
||
| You can choose any name for your changelog files because the GitHub action will rename files in | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but I mentioned it here
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't say you can't create mmultiple files. |
||
| `changelog/unreleased/features` and `changelog/unreleased/bugfixes` directories to `${PR_NUMBER}.md` when you open a PR. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the issues directory and just
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need PR's links only for bugfixes and features |
||
|
|
||
| Every push to the main or release branch Assemble changelog GitHub action will be executed: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The temporary files will be deleted, right?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, let's keep they before the release
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will those commit trigger metrics run? For now it seems so, because each PR merge produces 2 commits in the main branch.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [skip ci] tag allows skipping CircleCI jobs and GitHub Actions |
||
|
|
||
| * collect all files from `changelog/unreleased` | ||
| * assemble the changelog like: | ||
|
|
||
| ``` | ||
| #### Features | ||
| - Feature 1 [#1234](https://github.com/mapbox/mapbox-navigation-android/pull/1234) | ||
| - Feature 2 [#2345](https://github.com/mapbox/mapbox-navigation-android/pull/2345) | ||
|
|
||
| #### Bug fixes and improvements | ||
| - Bugfix 3 [#3456](https://github.com/mapbox/mapbox-navigation-android/pull/3456) | ||
| - Bugfix 4 [#4567](https://github.com/mapbox/mapbox-navigation-android/pull/4567) | ||
|
|
||
| #### Known issues :warning: | ||
| - Issue 1 | ||
| - Issue 2 | ||
|
|
||
| Some other changes | ||
| ``` | ||
|
|
||
| * write the changelog to the `changelog/unreleased/CHANGELOG.md` file | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about the release process? Shouldn't we move the changelog from unreleased directory to the project root and delete the unreleased one?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added more information |
||
|
|
||
| Every release the release train app will: | ||
|
|
||
| * get changelog from `changelog/unreleased/CHANGELOG.md` file | ||
| * add information about dependencies and compile changelog like: | ||
| ``` | ||
| ## Mapbox Navigation SDK 1.1.1 - 13 December, 2022 | ||
| ### Changelog | ||
| [Changes between v1.1.0 and v1.1.1](https://github.com/mapbox/mapbox-navigation-android/compare/v1.1.0...v1.1.1) | ||
|
|
||
| #### Features | ||
| - Feature 1 [#1234](https://github.com/mapbox/mapbox-navigation-android/pull/1234) | ||
| - Feature 2 [#2345](https://github.com/mapbox/mapbox-navigation-android/pull/2345) | ||
|
|
||
| #### Bug fixes and improvements | ||
| - Bugfix 3 [#3456](https://github.com/mapbox/mapbox-navigation-android/pull/3456) | ||
| - Bugfix 4 [#4567](https://github.com/mapbox/mapbox-navigation-android/pull/4567) | ||
|
|
||
| #### Known issues :warning: | ||
| - Issue 1 | ||
| - Issue 2 | ||
|
|
||
| Some other changes | ||
|
|
||
| ### Mapbox dependencies | ||
| This release depends on, and has been tested with, the following Mapbox dependencies: | ||
| - Mapbox Maps SDK `v10.8.0` ([release notes](https://github.com/mapbox/mapbox-maps-android/releases/tag/v10.8.0)) | ||
| - Mapbox Navigation Native `v115.0.1` | ||
| - Mapbox Core Common `v23.0.0` | ||
| - 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)) | ||
| ``` | ||
| * add compiled changelog to `CHANGELOG.md` file | ||
| * delete all files in `changelog/unreleased` dir | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @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 |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| - :warning: Updated the `NavigationView` default navigation puck asset. | ||
|
|
||
| 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: | ||
| ```kotlin | ||
| 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() | ||
| } | ||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fixed a rare `java.lang.NullPointerException: Attempt to read from field 'SpeechAnnouncement PlayCallback.announcement' on a null object reference` crash in `PlayCallback.getAnnouncement`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Added guarantees that route progress with `RouteProgress#currentState == OFF_ROUTE` arrives earlier than `NavigationRerouteController#reroute` is called. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fixed an issue where the first voice instruction might have been played twice. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Introduced `NavigationViewListener.onSpeedInfoClicked` that would be triggered when `MapboxSpeedInfoView` is clicked upon. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| - Fixed standalone `MapboxManeuverView` appearance when the app also integrates Drop-In UI. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import argparse | ||
| import os | ||
|
|
||
| parser = argparse.ArgumentParser(description='Add a new changelog file') | ||
|
|
||
| parser.add_argument('-f', '--feature', nargs='+', help='Features') | ||
| parser.add_argument('-b', '--bugfix', nargs='+', help='Bug fixes and improvements') | ||
| parser.add_argument('-i', '--issue', nargs='+', help='Known issues :warning:') | ||
| parser.add_argument('-o', '--other', nargs='+', help='Other changes') | ||
|
|
||
| args = parser.parse_args() | ||
|
|
||
|
|
||
| def write_file(changes, dir): | ||
| filename = 'changelog/unreleased/' + dir + '/changes.md' | ||
| os.makedirs(os.path.dirname(filename), exist_ok=True) | ||
| prepared_changes = '' | ||
| for change in changes: | ||
| prepared_changes += '- ' + change + '\n' | ||
| open(filename, 'w').write(prepared_changes) | ||
|
|
||
|
|
||
| if args.feature: | ||
| write_file(args.feature, 'features') | ||
|
|
||
| if args.bugfix: | ||
| write_file(args.bugfix, 'bugfixes') | ||
|
|
||
| if args.issue: | ||
| write_file(args.issue, 'issues') | ||
|
|
||
| if args.other: | ||
| write_file(args.other, 'other') |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import os | ||
|
|
||
| import git | ||
|
|
||
|
|
||
| def get_changes(path): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 🙂
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, good point, let's add tests for this logic |
||
| changes = '' | ||
| if not os.path.isdir(path): | ||
| return '' | ||
| files = os.listdir(path) | ||
| for file in files: | ||
| pr_number = file.partition('.')[0] | ||
| pr_changes = open(path + file, 'r').read() | ||
| if path.endswith('bugfixes/') or path.endswith('features/'): | ||
| pr_link = ' [#' + pr_number + '](https://github.com/mapbox/mapbox-navigation-android/pull/' + pr_number + ')' + '\n' | ||
| lines_with_description = [] | ||
| for line in open(path + file, 'r').readlines(): | ||
| if line.startswith('- '): | ||
| lines_with_description.append(line) | ||
| for line in lines_with_description: | ||
| pr_changes = pr_changes.replace(line, line.replace('\n', '') + pr_link) | ||
| if not pr_changes.endswith('\n'): | ||
| pr_changes += '\n' | ||
| changes += pr_changes | ||
| return changes.strip() | ||
|
|
||
|
|
||
| bugfixes = get_changes('changelog/unreleased/bugfixes/') | ||
| features = get_changes('changelog/unreleased/features/') | ||
| issues = get_changes('changelog/unreleased/issues/') | ||
| other = get_changes('changelog/unreleased/other/') | ||
|
|
||
| changelog = '#### Features\n' + features + '\n\n' + \ | ||
| '#### Bug fixes and improvements\n' + bugfixes + '\n\n' + \ | ||
| '#### Known issues :warning:\n' + issues + '\n\n' + \ | ||
| '#### Other changes\n' + other | ||
|
|
||
| old_changelog = open('changelog/unreleased/CHANGELOG.md', 'r').read() | ||
|
|
||
| if changelog != old_changelog: | ||
| open('changelog/unreleased/CHANGELOG.md', 'w').write(changelog) | ||
| repository = git.Repo('.') | ||
| repository.git.add('changelog/unreleased') | ||
| repository.index.commit('Assemble changelog file [skip ci]') | ||
| repository.remotes.origin.push().raise_if_error() | ||
|
|
||
| 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_old_changelog = open('libnavui-androidauto/changelog/unreleased/CHANGELOG.md', 'r').read() | ||
|
|
||
| if auto_changelog != auto_old_changelog: | ||
| open('libnavui-androidauto/changelog/unreleased/CHANGELOG.md', 'w').write(auto_changelog) | ||
| repository = git.Repo('.') | ||
| repository.git.add('libnavui-androidauto/changelog/unreleased') | ||
| repository.index.commit('Assemble auto changelog file [skip ci]') | ||
| repository.remotes.origin.push().raise_if_error() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import os | ||
| import re | ||
|
|
||
| import git | ||
|
|
||
| pr_number = os.environ['PR_NUMBER'] | ||
| token = os.environ['GITHUB_TOKEN'] | ||
|
|
||
| pattern = re.compile("^\d*.md") | ||
|
|
||
|
|
||
| def rename_files(path): | ||
| if not os.path.isdir(path): | ||
| return 0 | ||
|
|
||
| renamed_files_count = 0 | ||
| files = os.listdir(path) | ||
|
|
||
| new_md_files = list(filter(lambda file: not pattern.match(file), files)) | ||
|
|
||
| if len(new_md_files) > 1: | ||
| raise Exception('More than one new changelog file') | ||
|
|
||
| for file in new_md_files: | ||
| if not pattern.match(file): | ||
| os.rename(path + file, path + pr_number + '.md') | ||
| renamed_files_count += 1 | ||
|
|
||
| return renamed_files_count | ||
|
|
||
|
|
||
| renamed_bugfixes_count = rename_files('changelog/unreleased/bugfixes/') | ||
| renamed_features_count = rename_files('changelog/unreleased/features/') | ||
|
|
||
| auto_renamed_bugfixes_count = rename_files('libnavui-androidauto/changelog/unreleased/bugfixes/') | ||
| auto_renamed_features_count = rename_files('libnavui-androidauto/changelog/unreleased/features/') | ||
|
|
||
| if renamed_features_count + renamed_bugfixes_count + auto_renamed_bugfixes_count + auto_renamed_features_count > 0: | ||
| repository = git.Repo('.') | ||
| repository.git.add('changelog/unreleased') | ||
| repository.git.add('libnavui-androidauto/changelog/unreleased') | ||
| repository.index.commit('Rename changelog files') | ||
| repository.remotes.origin.push().raise_if_error() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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: