mapbox-navigation-android icon indicating copy to clipboard operation
mapbox-navigation-android copied to clipboard

6080 improve changelog verification

Open dzinad opened this issue 3 years ago • 4 comments

Closes #6080.

  1. Fixed the PR link check: before it checked that there's a PR link in the diff, while diff can be sth like:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index d5960be9d68..4a5bd3d04af 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,7 @@ Mapbox welcomes participation and contributions from everyone.
 
 ## Unreleased
 #### Features
+- Added ddtest.
 - Added `ViewBinderCustomization.infoPanelBinder` that allows installation of a custom info panel layout in `NavigationView`. [#6049](https://github.com/******/******-navigation-android/pull/6049) 
 - Added `ViewStyleCustomization.infoPanelPeekHeight` that allows customization of `NavigationView` info panel bottom sheet peek height. [#6049](https://github.com/******/******-navigation-android/pull/6049) 
 - Added `ViewStyleCustomization.infoPanelMarginStart`, `ViewStyleCustomization.infoPanelMarginEnd`, `ViewStyleCustomization.infoPanelBackground` that allows customization of a `NavigatioView` default info panel margins and background. [#6049](https://github.com/******/******-navigation-android/pull/6049)

So it didn't actually check the added lines. 2. Added the check to verify that the added line is in the 'Unreleased' section. 3. Added the check to verify that the added line is not contained changelog for any other stable version.

Notes: For now I left one "skip-changelog" label for everything. It disables all checks. If anyone thinks we need more precise labels (to skip the first check, or the second one, etc.), let's discuss.

dzinad avatar Jul 21 '22 12:07 dzinad

Codecov Report

Merging #6083 (d29f755) into main (73f0ea7) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #6083   +/-   ##
=========================================
  Coverage     68.60%   68.60%           
  Complexity     4185     4185           
=========================================
  Files           628      628           
  Lines         25341    25341           
  Branches       2977     2977           
=========================================
  Hits          17385    17385           
  Misses         6814     6814           
  Partials       1142     1142           

codecov[bot] avatar Jul 21 '22 12:07 codecov[bot]

@dzinad , what do you think about adding some tests which verifies logic of analysing changelog diff?

I do not suggest running those tests on CI, I'm thinking about one more py file which tests complex functions from scripts/validate-changelog.py so that I can run tests locally when I edit the script.

VysotskiVadim avatar Jul 25 '22 14:07 VysotskiVadim

@dzinad , what do you think about adding some tests which verifies logic of analysing changelog diff? I do not suggest running those tests on CI, I'm thinking about one more py file which tests complex functions from scripts/validate-changelog.py so that I can run tests locally when I edit the script.

Added some.

dzinad avatar Jul 25 '22 19:07 dzinad

What do you think about more complex system for labels: "Skip changelog" - verifies that changelog file wasn't modified "Modified changelog" - lets you modify existing release, but script verifies links "Ignore changelog erros" - turns off all checks ? BTW, this could be an overkill, current solution covers main flow, consider it as an idea 🙂

Yes, I think it's an overkill. More details here.

dzinad avatar Jul 26 '22 15:07 dzinad