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

[FEATURE] Introduce SpecificationVersion class #431

Merged
merged 10 commits into from
Nov 28, 2022
Merged

Conversation

larskissel
Copy link
Member

Introduce a utility module which handles SpecVersion
comparators for an easier SpecVersion maintainance
within the UI5 Tooling source code.
This utility is a kind of semver compatibility layer which
allows using semver functions with the SpecVersion
typical format "Major.Minor".

JIRA: CPOUI5FOUNDATION-224

@coveralls
Copy link

coveralls commented Nov 19, 2021

Coverage Status

Coverage increased (+0.1%) to 94.71% when pulling f1e97b7 on SpecVersionUtil into 89ae940 on main.

@matz3 matz3 changed the base branch from master to next November 22, 2021 08:35
RandomByte
RandomByte previously approved these changes Nov 30, 2021
Copy link
Member

@RandomByte RandomByte left a comment

Choose a reason for hiding this comment

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

LGTM. But as discussed, let's not merge this yet and wait for the consuming part to be implemented.

@flovogt flovogt changed the base branch from next to main November 3, 2022 13:30
@RandomByte RandomByte changed the title [FEATURE] Introduce SpecVersionUtil [FEATURE] Introduce SpecVersionComparator Nov 17, 2022
@RandomByte RandomByte force-pushed the SpecVersionUtil branch 2 times, most recently from ba1fce7 to f01bb21 Compare November 17, 2022 10:51
@RandomByte
Copy link
Member

I changed the implementation towards a "SpecVersionComparator" class, which is instantiated with a given Specification Version and can be used to compare that to other versions or ranges.

Instances of the class can then be provided directly by project-instances (getSpecVersionComparator()). Allowing consumers to not have to handle the project's specVersion themselves.

Copy link
Member

@matz3 matz3 left a comment

Choose a reason for hiding this comment

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

Maybe not a [FEATURE], as this is (currently) just used internally, right?

@RandomByte
Copy link
Member

The Specification.js getSpecVersionComparator function is public though 🤔

matz3
matz3 previously approved these changes Nov 24, 2022
Copy link
Member

@matz3 matz3 left a comment

Choose a reason for hiding this comment

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

Okay, true

larskissel and others added 4 commits November 24, 2022 17:25
Introduce a utility module which handles SpecVersion
comparators for an easier SpecVersion maintainance
within the UI5 Tooling source code.
This utility is a kind of semver compatibility layer which
allows using semver functions with the SpecVersion
typical format "Major.Minor".

JIRA: CPOUI5FOUNDATION-224
Static methods now create an instance and always throw for unsupported
versions (except isSupportedSpecVersion)
@RandomByte
Copy link
Member

Rebased onto main

lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
lib/specifications/utils/SpecVersionComparator.js Outdated Show resolved Hide resolved
Co-authored-by: Günter Klatt <57760635+KlattG@users.noreply.github.com>
@@ -151,6 +143,17 @@ class Specification {
return this._specVersion;
}

/**
* Returns an instance of a helper class allowing for convenient comparison
* operations against this instance's Specification Version
Copy link
Contributor

Choose a reason for hiding this comment

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

Does everybody know what a Specification Version is? I'm wondering if this whole versioning business is explained somewhere in the documentation.

Copy link
Member

Choose a reason for hiding this comment

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

We have a chapter in our documentation regarding that: https://sap.github.io/ui5-tooling/pages/Configuration/#specification-versions

KlattG
KlattG previously approved these changes Nov 25, 2022
@@ -309,12 +309,13 @@ class TaskRunner {
params.dependencies = dependencies;
}

if (task.getSpecVersion() === "3.0") {
const specVersion = task.getSpecVersionComparator();
Copy link
Member

@matz3 matz3 Nov 28, 2022

Choose a reason for hiding this comment

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

I struggle a bit with the naming of both the class and the getter. Here it's used with specVersion as local variable which reads fine (specVersion is greater then or equal to 3.0). But specVersionComparator.gte("3.0") would be strange.
But I also don't have a better idea, also as getSpecVersion() already returns the string. My only idea is to combine both to have getSpecVersion().toString() and getSpecVersion().gte("3.0")

Copy link
Member

@RandomByte RandomByte Nov 28, 2022

Choose a reason for hiding this comment

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

I'm not happy with the naming either.

I guess we could make this a SpecificationVersion class and have getSpecVersion() always return an instance of it. Indeed that might make things a bit simpler in the end.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that sounds good 👍🏻

Copy link
Member

Choose a reason for hiding this comment

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

Done in latest commit. I think this is way better, since the main use of a project's spec version is to compare it anyways 👍

Use it as the main representation of a specification version (instead of
the raw string as before). This also helps motivate the use of the
helper functions to compare the version. Which is the most common use of
the specification version anyways.
RandomByte added a commit to SAP/ui5-builder that referenced this pull request Nov 28, 2022
RandomByte added a commit to SAP/ui5-tooling that referenced this pull request Nov 28, 2022
With the latest changes in SAP/ui5-project#431,
the method now returns an instance of a "SpecificationVersion" class.
This would require an additional specVersion-dependent interface.

Since use cases in custom tasks and -middelare are not clear, we should
rather keep it simple for now and do not provide this method.
@RandomByte RandomByte changed the title [FEATURE] Introduce SpecVersionComparator [FEATURE] Introduce SpecificationVersion class Nov 28, 2022
@RandomByte RandomByte merged commit e57842b into main Nov 28, 2022
@RandomByte RandomByte deleted the SpecVersionUtil branch November 28, 2022 15:03
RandomByte added a commit to SAP/ui5-builder that referenced this pull request Nov 29, 2022
RandomByte added a commit to SAP/ui5-tooling that referenced this pull request Jan 24, 2023
With the latest changes in SAP/ui5-project#431,
the method now returns an instance of a "SpecificationVersion" class.
This would require an additional specVersion-dependent interface.

Since use cases in custom tasks and -middelare are not clear, we should
rather keep it simple for now and do not provide this method.
d3xter666 pushed a commit that referenced this pull request Feb 7, 2023
Introduce a helper class representing a Specification Version.
It provides methods for comparing versions, which simplifies
version checks across the UI5 Tooling source code.

The methods act as a kind of semver-adapter which ultimately
allows executing specific semver functions on the SpecVersion
format "Major.Minor".

Co-authored-by: Günter Klatt <57760635+KlattG@users.noreply.github.com>
Co-authored-by: Merlin Beutlberger <m.beutlberger@sap.com>
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.

5 participants