Verify locally installed Git version is >=2.11 #349
Conversation
| }; | ||
|
|
||
| exports.verifyRecentGitVersion = async () => { | ||
| if (semver.lt(await gitVersion(), '2.11.0')) { |
There was a problem hiding this comment.
I just noticed that this could be moved to a separate method in version.js named isVersionLower, along with isVersionGreater. Doing so would keep this file (almost) entirely focused on git-related methods.
Also, we're currently not checking if the output returned by gitVersion is actually a version, which could lead to some issues in future versions of git.
@sindresorhus Thoughts?
There was a problem hiding this comment.
isVersionLower, along withisVersionGreater.
We don't really need both of these though. How about just one called isVersionGreaterThanOrEqualTo()? That's a mouthful though... Alternatively, isMinimumVersion() / isAtLeastVersion().
Also, we're currently not checking if the output returned by
gitVersionis actually a version, which could lead to some issues in future versions of git.
We should definitely do that.
There was a problem hiding this comment.
We don't really need both of these though. How about just one called
isVersionGreaterThanOrEqualTo()? That's a mouthful though... Alternatively,isMinimumVersion()/isAtLeastVersion().
In ui.js and prerequisite-tasks.js we're using !isVersionGreater(), which is the same as isVersionLowerThanOrEqualTo(). This requires us to have a way to check if v1 >= v2 as well as if v1 <= v2, which means we'll need to have at least 2 methods.
To solve this, we could keep isVersionGreater() and add another method for checking version equality isVersionEqualTo() (which is more readable in my opinion).
There was a problem hiding this comment.
Sounds good. Then we should also rename isVersionGreater() => isVersionGreaterThan() for consistency.
This fixes #348.