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

Move to semver versioning for MO2. #154

Merged
merged 5 commits into from
Aug 5, 2024
Merged

Move to semver versioning for MO2. #154

merged 5 commits into from
Aug 5, 2024

Conversation

Holt59
Copy link
Member

@Holt59 Holt59 commented Jun 30, 2024

This PR adds functions related to Semantic Version parsing/comparing to uibase (https://semver.org/).

Why?

I started working on this for extensions but in the end I feel like this can be integrated sooner, hence the PR.

To me the current VersionInfo is very flawed, and hard to work with, moving to a more streamlined versioning scheme will probably help further development.

The goal is not to follow directly the semver scheme of major/minor/patch for breaking changes, etc., but rather to have a streamlined and specified way of parsing/presenting and comparing versions.

Impact?

This should have little impact on code and user-experience, aside from MO2 itself (I will make a corresponding PR) -

  • this will not change versioning stuff for mods, games, etc. - Nexus has no streamlined versioning scheme so trying to fit it into semver does not feel like a good idea and may have huge impact, so currently out-of-scope
  • this will change the version of MO2 to use a new Version object - the old appVersion() method from IOrganizer will simply be deprecated but the underlying implementation will actually use version().

Note: This also changes the layout structure of code in VS because the flat layout was getting a bit hard to navigate. I did not feel like making a separate PR for it.

src/version.h Outdated Show resolved Hide resolved
@AnyOldName3
Copy link
Member

As it's not super clear from the PR description, are we planning on treating the 2 in MO2 as the semver major version, and having MO3 the next time there's a breaking change to the plugin API, having the 5 in MO2 2.5.x be the semver major version and having MO2 2.6 the next time there's a breaking change to the plugin API, or something else?

@Holt59
Copy link
Member Author

Holt59 commented Jun 30, 2024

As it's not super clear from the PR description, are we planning on treating the 2 in MO2 as the semver major version, and having MO3 the next time there's a breaking change to the plugin API, having the 5 in MO2 2.5.x be the semver major version and having MO2 2.6 the next time there's a breaking change to the plugin API, or something else?

The changes is more about enforcing the semver syntax on versions (for MO2, and for extensions in the future) rather than to enforce the actual major/minor/patch semantic, although if we go that way, I'd say that minor would be breaking changes and patch bug fix.

@Al12rs
Copy link
Member

Al12rs commented Jul 2, 2024

Does this have any chance of breaking version migration code?

We have code that compares current version and previously installed version to determine whether some version migration code needs to be executed.

@Holt59
Copy link
Member Author

Holt59 commented Jul 2, 2024

Does this have any chance of breaking version migration code?

We have code that compares current version and previously installed version to determine whether some version migration code needs to be executed.

The migration code actually uses QVersionNumber, so this should not break anything since I just change the conversion from VersionInfo to Version, you can check https://github.com/ModOrganizer2/modorganizer/pull/2063/files#diff-c72b037f3ac7abbd3d0ee519b6f456013a9bbf5a59df16308939320c04970447R2179

I tested the changes with various MO2 version by modifying the version.rc, and everything is working fine. There are some workaround to deal with subsubminor versions that are not really handled by semver but these are for kind of old MO2 releases so should not really matter, it's more to avoid warning when parsing the list of releases from Github.

MO2 uses its own version string, which are not semver compliant, so what I did is add a custom parser for those - you can see there is a ParseMode option to Version::parse.

You can also check the other PR ModOrganizer2/modorganizer#2063

@Holt59
Copy link
Member Author

Holt59 commented Jul 2, 2024

Does this have any chance of breaking version migration code?

We have code that compares current version and previously installed version to determine whether some version migration code needs to be executed.

The only changes from a user/"behavior" point-of-view would be the display of the version for users, but only for non-final releases. Before it would have been 2.4.1a4 or 2.5.2rc1, after it will be 2.4.1-alpha.4 or 2.5.2-rc.1. It would not be complicated to have Version::string take an optional argument to get formatting as before, but I do not think it would really be useful.

@Holt59
Copy link
Member Author

Holt59 commented Jul 18, 2024

I have changed the PR to allow a "subpatch" in the versions, so that 2.2.2.1 would not be considered a pre-release of 2.2.2. This should only be used for MO2. I updated the comment of the Version class and the ParseMode enum.

I also added configurations for the string() function that can be used to change the display so that display matches the old one, I'll update the MO2 PR to match these.

@Holt59 Holt59 mentioned this pull request Aug 5, 2024
5 tasks
@Holt59 Holt59 merged commit 7e13c48 into dev/vcpkg Aug 5, 2024
4 checks passed
@Holt59 Holt59 deleted the dev/semver branch August 5, 2024 09:40
Holt59 added a commit that referenced this pull request Aug 9, 2024
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.

3 participants