acquia / cli

Command-line interface for Acquia Cloud Platform products
https://docs.acquia.com/acquia-cli/
GNU General Public License v2.0
42 stars 45 forks source link

GL-2753 : CS Wizard updated node version select values #1764

Closed PrakharJainS3 closed 1 week ago

PrakharJainS3 commented 1 week ago

Motivation

The current values provided in NODE_VERSION need to be update as for better onboarding flow to user on Node Project. Also the default value for the select needed to be changed.

Testing steps

Following steps can be taken to verify the changes:

codecov[bot] commented 1 week ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 91.75%. Comparing base (a845434) to head (5d215b3).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1764 +/- ## ========================================= Coverage 91.75% 91.75% Complexity 1818 1818 ========================================= Files 121 121 Lines 6524 6524 ========================================= Hits 5986 5986 Misses 538 538 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 1 week ago

Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/1764/acli.phar

curl -OL https://acquia-cli.s3.amazonaws.com/build/pr/1764/acli.phar
chmod +x acli.phar
anavarre commented 1 week ago

Typically, Node versions are updated quite frequently upstream. It means you're gonna have to change the versions in ACLI equally often and get a new release out accordingly. Do we not have something more sustainable with less toil to envision?

PrakharJainS3 commented 1 week ago

That is what we are aiming for using these changes. So previously we had minor as well as patch version selected by user in the CLI, but now we will only provide the option to select the major version which changes in a long time. We will manage the precise version to be selected by us in the background that can be changed by the user later if needed.

anavarre commented 1 week ago

Believe it or not, I actually looked at the diff the other way around. All good!

danepowell commented 1 week ago

Looks good except for the failing mutation test. It's telling you that tests still pass when Node 18 is removed from the available options. Looks like you need to add/improve the test for Node 18.

danepowell commented 1 week ago

Note that the coding standards on this project changed dramatically since you opened this PR. You'll need to rebase or merge on master, and it might be easier to just start a new PR.

PrakharJainS3 commented 1 week ago

Will open a new PR.