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

Use same version number for web and desktop #932

Merged
merged 4 commits into from
Sep 5, 2024

Conversation

kevinschaul
Copy link
Collaborator

With the new release process in place, I think keeping a separate version number for desktop is confusing and unnecessary. This PR remove it so the desktop version reports the version number of the web version embedded in the binary.

Launch Checklist

  • Briefly describe the changes in this PR.
  • Add an entry to CHANGELOG.md under the ## main section.

With the new release process in place, I think keeping a separate
version number for desktop is confusing and unnecessary. This PR remove
it so the desktop version reports the version number of the web version
embedded in the binary.
Copy link
Collaborator

@HarelM HarelM left a comment

Choose a reason for hiding this comment

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

Sounds right to me.

@HarelM
Copy link
Collaborator

HarelM commented Sep 3, 2024

I know that in windows you can check excitable file properties and see a version number. Is this number shown there?

@kevinschaul
Copy link
Collaborator Author

@HarelM that's a good question. I do not think the version number would be included there currently. A quick search reveals a few tools to embed that information, e.g. https://github.com/tc-hib/go-winres. But unfortunately I do not have access to a Windows machine, so I would be unable to test this.

@HarelM
Copy link
Collaborator

HarelM commented Sep 3, 2024

I can check if you upload it, if that helps.

@kevinschaul
Copy link
Collaborator Author

@HarelM Can you check this artifact on Windows? https://github.com/maplibre/maputnik/actions/runs/10707430874?pr=932

@HarelM
Copy link
Collaborator

HarelM commented Sep 4, 2024

Looks good! the only comment I have is regarding the icon (top left corner in the following screenshot), would be nice if it had the Maputnik icon for it instead of the default windows exe icon.
As far as I can tell this can be defined here: https://github.com/tc-hib/go-winres?tab=readme-ov-file#icon-json
image
Overall, this is approved.

@kevinschaul
Copy link
Collaborator Author

@HarelM
Copy link
Collaborator

HarelM commented Sep 5, 2024

This looks great! Thanks!
image
It might benefit from a larger icon maybe as when you increase it wildly it looks like so (see the PDF for comparison):
image

Another side note is that the following is the desktop experience basically:
image

I was under the impression that this will be more of an electron type of desktop application.
But since this is not the case, I would suggest opening the browser if possible in the given address at least.

In any case all of those are in the nice to have realm...

Thanks for all the work around this!

@kevinschaul
Copy link
Collaborator Author

Interesting, I think I'll leave the icon as-is. Good nice-to-have for the future, though. And agreed that it should open the browser. I'll add that as a new issue and hopefully get to it soon! Thank you!

@kevinschaul kevinschaul merged commit d940c02 into maplibre:main Sep 5, 2024
7 checks passed
@kevinschaul kevinschaul deleted the single-version-number branch September 5, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants