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

Replace --production by --omit=dev #1120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

swaldmann
Copy link

@swaldmann swaldmann commented Jun 25, 2024

Description

This PR replaces all occurrences of --production in an npm context with --omit=dev.

Currently you get these warnings when deploying MTA projects with the standard npm builder:
"npm warn config production Use --omit=dev instead"

The omit option was introduced with npm 8, so it's available in all supported versions.

Checklist

  • Code compiles correctly
  • Relevant tests were added (unit / contract / integration)
  • Relevant logs were added
  • Formatting and linting run locally successfully
  • All tests pass
  • UA review
  • Design is documented
  • Extended the README / documentation, if necessary
  • Open source is approved

Copy link

cla-assistant bot commented Jun 25, 2024

CLA assistant check
All committers have signed the CLA.

@swaldmann swaldmann marked this pull request as ready for review June 25, 2024 10:54
@swaldmann swaldmann requested a review from wangshen-sap as a code owner June 25, 2024 10:54
@yutaoj
Copy link
Collaborator

yutaoj commented Aug 8, 2024

this parameter "--production" is still used by Node v14 .

@swaldmann
Copy link
Author

Node 14 (and 16) are already end-of-life though. They shouldn't be used any more, as they won't even get patched. IMO you should drop support for them, as in the worst case this enables stakeholders using those outdated versions.

Even if Node 14 support has to be kept for some reason there should be a conditional to use the --omit=dev version for later Node versions. We really shouldn't show warnings for standard projects using a current LTS version just to accommodate to some version deprecated for years.

@jerome-benoit
Copy link
Collaborator

jerome-benoit commented Aug 8, 2024

Even if Node 14 support has to be kept for some reason there should be a conditional to use the --omit=dev version for later Node versions. We really shouldn't show warnings for standard projects using a current LTS version just to accommodate to some version deprecated for years.

If you expect that repo to follow the most basic best current security practices or even SAP security policies, you will face disillusionment :) I've tried to push a bunch of security compliance PRs a year ago, most of them have been merged/taken over.

Dunno why such a critical piece in the SAP software supply chain can be left with known critical CVEs such as https://security-tracker.debian.org/tracker/CVE-2024-2961 several months ... or years.

@yutaoj
Copy link
Collaborator

yutaoj commented Nov 8, 2024

MBT requires support for Node 14, and the Node 14 MBT Docker image is utilized by SAP Piper. Therefore, it cannot be replaced at this time.

@jerome-benoit
Copy link
Collaborator

So critical components in the SAP software supply chain use unmaintained and cluttered by serious security flaws node.js version?
How can it be considered as a valid justification to continue to support them in another SAP software supply chain critical component?

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.

3 participants