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

fix: reset script from package.json does not work #1656 #1848

Merged
merged 1 commit into from
Feb 25, 2022

Conversation

ruzell22
Copy link
Contributor

fixes: #1656

Signed-off-by: ruzell22 ruzell.vince.aquino@accenture.com

@ruzell22
Copy link
Contributor Author

@petermetz , Upon reading further about yarn.lock, del-cli yarn.lock wasn’t needed and may cause problems. Letting yarn update the yarn.lock file is the better way in terms of updating dependencies.
(https://classic.yarnpkg.com/en/docs/yarn-lock/#toc-managed-by-yarn : # Managed by Yarn)
(https://stackoverflow.com/questions/60509429/is-it-a-good-idea-to-delete-yarn-lock-and-generate-it-again-by-running-yarn-inst)
(https://stackoverflow.com/questions/41126217/how-to-sync-yarn-lock-with-package-json)

Testing has also been done in terms of using “del-cli ‘**/node_modules’” and reinstalling + reconfiguring it.
yarn del-cli emptied node_modules (on the left side)
image

yarn install --update-checksum --force checks yarn.lock and updates it necessarily and also installs node_modules together
(https://classic.yarnpkg.com/en/docs/cli/install/)
(https://stackoverflow.com/questions/41864099/how-do-i-force-yarn-to-reinstall-a-package)
(https://www.codegrepper.com/code-examples/shell/yarn+reinstall+all+packages)
image

yarn configure configured the package and outputs all lerna success
image

Yarn reset is also reusable even after using yarn reset once unlike the other solutions I have tried.

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@petermetz , Upon reading further about yarn.lock, del-cli yarn.lock wasn’t needed and may cause problems. Letting yarn update the yarn.lock file is the better way in terms of updating dependencies.
(https://classic.yarnpkg.com/en/docs/yarn-lock/#toc-managed-by-yarn : # Managed by Yarn)
(https://stackoverflow.com/questions/60509429/is-it-a-good-idea-to-delete-yarn-lock-and-generate-it-again-by-running-yarn-inst)
(https://stackoverflow.com/questions/41126217/how-to-sync-yarn-lock-with-package-json)

Testing has also been done in terms of using “del-cli ‘**/node_modules’” and reinstalling + reconfiguring it.
yarn del-cli emptied node_modules (on the left side)

yarn install --update-checksum --force checks yarn.lock and updates it necessarily and also installs node_modules together
(https://classic.yarnpkg.com/en/docs/cli/install/)
(https://stackoverflow.com/questions/41864099/how-do-i-force-yarn-to-reinstall-a-package)
(https://www.codegrepper.com/code-examples/shell/yarn+reinstall+all+packages)

yarn configure configured the package and outputs all lerna success

Yarn reset is also reusable even after using yarn reset once unlike the other solutions I have tried.

@ruzell22 Sure thing, please also amend the commit message to contain this description of the solution and the reasoning behind it (screenshots can be omitted). That way if we ever choose to migrate away from github people can still look back in the git history itself and see why we were doing what we are. Other than that, LGTM and thank you again!

@petermetz petermetz requested review from izuru0 and takeutak and removed request for jonathan-m-hamilton February 21, 2022 22:09
@ruzell22 ruzell22 force-pushed the resetscriptfix branch 2 times, most recently from c260ff1 to e011c85 Compare February 22, 2022 06:12
@ruzell22
Copy link
Contributor Author

@petermetz I have updated the commit message. Thank you!
image

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@ruzell22 Thank you very much, now the problem is that the commit message has lines longer than a 100 characters in the footer (see the commit lint CI check's logs for this below)

I recommend using rulers in VSCode while editing the commit message text so that you can always see where the 80/100/120 character lines are.

@ruzell22
Copy link
Contributor Author

@petermetz I have now fixed the commit body not exceeding 100 characters per line. Thank you.

Copy link
Contributor

@petermetz petermetz left a comment

Choose a reason for hiding this comment

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

@ruzell22 Great, thank you for the fix! LGTM

Copy link
Contributor

@izuru0 izuru0 left a comment

Choose a reason for hiding this comment

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

LGTM



fixes: hyperledger-cacti#1656

Upon reading further about yarn.lock, del-cli yarn.lock wasn't needed and may cause problems.
Letting yarn update the yarn.lock file is the better way in terms of updating dependencies.
(https://classic.yarnpkg.com/en/docs/yarn-lock/#toc-managed-by yarn : # Managed by Yarn)
(https://stackoverflow.com/questions/60509429)
(https://stackoverflow.com/questions/41126217)

Testing has also been done in terms of using "del-cli '**/node_modules'" and reinstalling it.
yarn del-cli emptied node_modules.
yarn install --update-checksum --force checks yarn.lock and updates it necessarily
and also installs node_modules together.
(https://classic.yarnpkg.com/en/docs/cli/install)
(https://stackoverflow.com/questions/41864099)
(https://www.codegrepper.com/code-examples/shell/yarn+reinstall+all+packages)

yarn configure configured the package and outputs all lerna success.
yarn reset is also reusable even after using yarn reset once.

Signed-off-by: ruzell22 <ruzell.vince.aquino@accenture.com>
@petermetz petermetz merged commit c74e002 into hyperledger-cacti:main Feb 25, 2022
@petermetz petermetz deleted the resetscriptfix branch February 25, 2022 18:24
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.

fix: reset script from package.json does not work
3 participants