Skip to content

Add isAtOrAbove and assertAtOrAbove#251

Open
Panman82 wants to merge 1 commit into
ember-cli:masterfrom
Panman82:at-or-above
Open

Add isAtOrAbove and assertAtOrAbove#251
Panman82 wants to merge 1 commit into
ember-cli:masterfrom
Panman82:at-or-above

Conversation

@Panman82

@Panman82 Panman82 commented Feb 9, 2021

Copy link
Copy Markdown

Currently the assertAbove() function is incorrectly documented as a way to check for a minimum version and is actually a footgun if used in this way. (Ask me how I know... LOL)

Throws an error with the given message if a minimum version isn't met.

Instead, you must specify the version prior to the minimum version you are concerned about. This can also become an issue if there is a patch version of the prior version, which would then be unexpectedly accepted by the check. Ex: Ember Octane (v3.15) is the "true" minimum version, so you assertAbove('3.14.2'), but then v3.14.3 comes out...

This PR clarifies the documentation for assertAbove() and isAbove() but also adds assertAtOrAbove() and isAtOrAbove(), with documentation and testing. Basically the difference between the two is gt() vs gte().

@Panman82

Copy link
Copy Markdown
Author

Thinking more about this, after looking at what semver offers, another option is to just add an assert function for each version comparison function that this addon exports. That way the custom names isAbove and isAtOrAbove do not need to defined here. Rather, just use one of the semver function and prefix assert to the name to get that functionality.

PS. I think that one test scenario just needs to be re-run, it looks like there was some network issue when it booted up.

@Panman82

Copy link
Copy Markdown
Author

Reviewing my forks... Two years since I submitted this, any interest? With the move to peerDependancies and the embroider stuff, I'm not sure that this addon is even needed now. 🤷‍♂️

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.

1 participant