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

Improve DetectUnpinnedDotnetToolInstallVersions function #110

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

tehraninasab
Copy link
Contributor

Add test and clean the code.

@knocte
Copy link
Member

knocte commented Jul 6, 2023

Maybe we need to expand this PR to also cover unpinned npm packages (e.g. see https://github.com/nodeeffect/RunIntoMe/commit/349e153aeb579ddc83a8a72a19e71480f26eca04 ).

@tehraninasab tehraninasab force-pushed the improveUnpinnedDotnetToolInstallVersions-squashed branch from e6c723d to ef3e8bb Compare July 6, 2023 09:19
Add test for DetectUnpinnedDotnetToolInstallVersions function.
Add failing test for detecting unpinned version in npm package
installations.
@tehraninasab tehraninasab force-pushed the improveUnpinnedDotnetToolInstallVersions-squashed branch from 8d0c2ac to 283602c Compare July 6, 2023 10:08
Implement DetectUnpinnedNpmPackageInstallVersions function.
@tehraninasab tehraninasab force-pushed the improveUnpinnedDotnetToolInstallVersions-squashed branch from 222cd21 to 5545a54 Compare July 6, 2023 10:28
Add another test for DetectUnpinnedNpmPackageInstallVersions.
@tehraninasab tehraninasab force-pushed the improveUnpinnedDotnetToolInstallVersions-squashed branch 6 times, most recently from 987d052 to 35616b9 Compare July 6, 2023 12:44
@tehraninasab
Copy link
Contributor Author

Maybe we need to expand this PR to also cover unpinned npm packages (e.g. see nodeeffect/RunIntoMe@349e153 ).

It's done

@@ -77,7 +77,7 @@ jobs:
sudo apt install --yes --no-install-recommends npm curl
# need to update nodejs because with ubuntu's default nodejs version we would get this error:
# error @jest/[email protected]: The engine "node" is incompatible with this module. Expected version "^14.15.0 || ^16.10.0 || >=18.0.0". Got "12.22.9"
sudo npm install --global n
sudo npm install --global n@9.1.0
Copy link
Member

Choose a reason for hiding this comment

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

@realmarv can we use stable instead?

Copy link
Contributor Author

@tehraninasab tehraninasab Jul 6, 2023

Choose a reason for hiding this comment

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

you mean @latest?

Copy link
Member

Choose a reason for hiding this comment

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

no, in fact if using @latest, this script should fail as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there is no @stable tag

@tehraninasab tehraninasab force-pushed the improveUnpinnedDotnetToolInstallVersions-squashed branch from 35616b9 to 61ed51e Compare July 13, 2023 12:54
Add unpinnedNpmPackageInstallVersions.fsx script and specify
package versions in npm install commands in GitHubCI.
Add failing test for DetectUnpinnedNpmPackageInstallVersions
function.
Fix DetectUnpinnedNpmPackageInstallVersions function.
Add failing test for DetectUnpinnedNpmPackageInstallVersions
function.
Fix DetectUnpinnedNpmPackageInstallVersions function.
@tehraninasab tehraninasab force-pushed the improveUnpinnedDotnetToolInstallVersions-squashed branch from 1433c8e to bd8be73 Compare July 13, 2023 13:24
@knocte knocte force-pushed the master branch 2 times, most recently from c5fb9d5 to 18898b7 Compare July 27, 2023 02:00
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.

2 participants