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

Node version check #305

Merged
merged 4 commits into from
Jul 24, 2020
Merged

Node version check #305

merged 4 commits into from
Jul 24, 2020

Conversation

rjherrera
Copy link
Member

Description

This PR adds a version check for the node version. Node is a dependency for potassium and project creation often broke due to node incompatibilities.

Details

  • The node version check is performed and communicated via exceptions. It can be disabled (at users own risk, for example to use a newer version of node) via the --no-node-version-check flag.
  • The way the potassium version was checked is also modified to fit the new exception-based flow.

@rjherrera rjherrera requested a review from ldlsegovia July 3, 2020 15:50
Copy link
Collaborator

@ldlsegovia ldlsegovia left a comment

Choose a reason for hiding this comment

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

Se me ocurre que quizás sería bueno tener una BaseVersionEnsurer del cual heredan las demás y en ese definir la interfaz. con un def ensure!; raise NotImplemented; end y no se que otra cosa más. Luego tenés un array de ensurers que iteras y cada uno recibe new(options) y corre ensure!. Como para dejar más explícito el cómo agregar un nuevo version ensurer.

Es sólo una idea (no sé si vale algo así de robusto para este asunto), creo que está 👌 pero para tener en cuenta

@rjherrera rjherrera merged commit 8a95c93 into master Jul 24, 2020
@rjherrera rjherrera deleted the node-version-check branch July 24, 2020 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants