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

breaking: drop node engine requirement #276

Merged
merged 2 commits into from
Jun 26, 2020

Conversation

erisu
Copy link
Member

@erisu erisu commented Jun 26, 2020

Motivation, Context & Description

  • In a production environment, our plugins do not execute node related code and does not need the engine check.
  • In the environment of developing this plugin, the linting tool is the only node based library which has already defined the node engine requirement. We do not need to set the engine since the lint tool will report, when not valid.

Example:

npm WARN notsup Unsupported engine for @cordova/eslint-config@3.0.0: wanted: {"node":">= 10.13.0"} (current: {"node":"8.17.0","npm":"6.13.4"})
npm WARN notsup Not compatible with your version of node/npm: @cordova/eslint-config@3.0.0

@erisu erisu added this to the 6.0.0 milestone Jun 26, 2020
@jcesarmobile
Copy link
Member

Can’t we just remove the node entry? Other plugins don’t have it (didn’t check all, but didn’t see in 3 random ones)

@erisu
Copy link
Member Author

erisu commented Jun 26, 2020

Yeah, we could probably remove the node engine from our plugins.

The only thing that has a node requirement is the linting tool, but it is only for development purposes.

Our plugins do not use any hook-scripts or node-specific code in production, so there is no node execution. Only the Cordova CLI tooling, which does the copying and merging of the src or www content, has the requirement.

I think it is safe to drop it.

@erisu erisu changed the title breaking: bump node support >=10.0.0 breaking: drop node engine Jun 26, 2020
@erisu erisu changed the title breaking: drop node engine breaking: drop node engine requirement Jun 26, 2020
@jcesarmobile
Copy link
Member

Yeah, I can check them all later, but so far I’ve checked 3 and they didn’t have it

@erisu erisu merged commit 369f8d1 into apache:master Jun 26, 2020
@erisu erisu deleted the breaking/bump-node-engine branch June 26, 2020 09:55
@jcesarmobile
Copy link
Member

I have reviewed all the plugins (even archived and deprecated ones) and none had a node requirement, so we are good

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants