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

CI: enable snapshot repo for snapshot releases #6870

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

RingerJK
Copy link
Contributor

Description

CI: enable snapshot repo for snapshot releases

⚠️ other jobs are failing: the changes allow snapshot release only, but not build, tests, analytics, etc on top of a snapshot

Ideally, the above jobs should be allowed but forbidden to merge.

@RingerJK RingerJK added the skip changelog Should not be added into version changelog. label Jan 26, 2023
@RingerJK RingerJK requested review from SevaZhukov and a team January 26, 2023 10:59
@RingerJK RingerJK self-assigned this Jan 26, 2023
@github-actions
Copy link

Changelog

Features

  • Added RoadComponent.language value. #6833
  • ⚠️ Changed EHorizonEdgeMetadata.names class from RoadName to RoadComponent. #6833
    To migrate change your code from:
val shielded = roadName.shielded

into:

val shielded = roadComponent.shield != null
  • Added support for continuous EV alternatives in NavigationRouteAlternativesObserver. #6833
  • Fixed issues with map-matching to HOV-only roads. #6833
  • Set a limit of simultaneously running onboard route requests to avoid too many tasks blocking too much of the device's computing resources. #6833
  • Fixed an issue with Road Access Policy ignoring the setting to map-match to closed road sections, when enabled. #6833
  • Added MapboxTripStarter to simplify the solution for managing the trip session and replaying routes. This also makes it possible to share the replay state between drop-in-ui and android-auto. #6794

Bug fixes and improvements

  • Increased max distance from the user indicator to route line valid to continue vanishing updates from 3m to 10m. #6854
  • Fixed an issue where alternative routes might have had an incorrect or incomplete portion of the route refreshed or occasionally fail to refresh. #6848
  • Removed NavigationRoute#hasUnexpectedClosures and added RouteProgress#hasUnexpectedUpcomingClosures instead that checks whether route has upcoming unexpected closures (the algorithm does not take into account closures that the puck has already been on) #6841
  • Improved NavigationView camera behavior to go back into overview state if routes change during route preview state. #6840
  • Improved NavigationView camera behavior to ignore keyboard insets. #6845

Known issues ⚠️

Other changes

Android Auto Changelog

Features

  • Added support for Junction Views. #6849

Bug fixes and improvements

  • Optimized SpeedLimitWidget memory usage. #6859

@codecov
Copy link

codecov bot commented Jan 26, 2023

Codecov Report

Merging #6870 (417d011) into main (09b5248) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #6870   +/-   ##
=========================================
  Coverage     72.68%   72.68%           
  Complexity     5572     5572           
=========================================
  Files           782      782           
  Lines         30166    30166           
  Branches       3562     3562           
=========================================
  Hits          21926    21926           
  Misses         6814     6814           
  Partials       1426     1426           

@LukasPaczos
Copy link

@RingerJK since using a snapshot shouldn't be allowed to be merged, this means that a snapshot with a snapshot can only be released from a non-protected/non-main branch. In this case, couldn't you enable snapshot builds directly in the branch and revert it before merging?

@RingerJK
Copy link
Contributor Author

In this case, couldn't you enable snapshot builds directly in the branch and revert it before merging?

@LukasPaczos I thought about it, but it seems it's not so straightforward: enabling a branch snapshot means we need an additional check that prevents merging this branch into the production branch (main/release-v*). So I would say the changes in the current PR are safe to merge, but what you proposed needs additional investigation. I would cut a ticket for @SevaZhukov for a pretty-looks solution

@LukasPaczos
Copy link

enabling a branch snapshot means we need an additional check that prevents merging this branch into the production branch

This should be rejected during a review process, as with any other change. I don't think adding the flag proposed in this PR makes that safer in any way.

@RingerJK
Copy link
Contributor Author

actually, cannot fully agree: the PR cannot be merged as 0 check is passed, so the PR checks be 🔴 , but snapshot release is 🍏

@VysotskiVadim
Copy link
Contributor

@LukasPaczos , do you worry that snapshot dependency could be accidentally used during normal release if this PR will be merged?

@RingerJK
Copy link
Contributor Author

Copy link

@LukasPaczos LukasPaczos left a comment

Choose a reason for hiding this comment

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

I don't have too strong of an opinion. The flag just looks a little bit redundant since you need to change one locally anyway, but if you see this improving some workflow, let's merge.

@RingerJK RingerJK merged commit 9b4793b into main Feb 6, 2023
@RingerJK RingerJK deleted the kyv-allow-snapshots-for-snapshot-releases branch February 6, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip changelog Should not be added into version changelog.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants