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

Revert "Encode the style URL in iOS (#2965)" #3024

Merged
merged 2 commits into from
Nov 17, 2024
Merged

Conversation

louwers
Copy link
Collaborator

@louwers louwers commented Nov 16, 2024

This reverts commit d140d5e.

This commit has caused several regressions #3023

Potentially also #2993

I want to revert it, put out a release, and then look for a fix.

@louwers louwers requested a review from HarelM November 16, 2024 20:40
@HarelM
Copy link
Collaborator

HarelM commented Nov 16, 2024

I would advise to split a revert PR with a version release PR, but that might be an overkill.
I think it helps to see in the git history a commit for "bump version" (besides the tagging of course) since then you can easily see which commit was added in which version without a lot of hassle.
I use an action to create a bump version PR, which also takes care of the change log modification. I think it's a nice way to solve this, also it allows me to release a version without any extra approval (which is nice by its own).

@louwers
Copy link
Collaborator Author

louwers commented Nov 16, 2024

@HarelM I agree that is a cleaner way to do it. I am guilty of lumping unrelated things into one PR, because CI for this repo is quite heavy and takes a long time to run. Not really a good reason though.

I usually update the changelog as part of the version bump PR. I also think that releasing a version should require an approval.

@louwers louwers self-assigned this Nov 16, 2024
@louwers louwers added the iOS label Nov 16, 2024
Copy link

Bloaty Results (iOS) 🐋

Compared to main

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  -0.0%    -144  [ = ]       0    TOTAL

Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results-ios/pr-3024-compared-to-main.txt

@HarelM
Copy link
Collaborator

HarelM commented Nov 16, 2024

If all you do is "push the release button" I don't see a lot of merit in approving this. But I guess it also depends on how fast you get a PR approval for something so trivial, which I find to long in my case...

@louwers louwers merged commit 6be18d2 into maplibre:main Nov 17, 2024
30 checks passed
@louwers louwers deleted the ios-6-8-1 branch November 17, 2024 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants