-
-
Notifications
You must be signed in to change notification settings - Fork 5
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
code cleanup, add unit tests, hook up eslint and prettier #100
Conversation
prefer squash :) Potentially pr title could be better 🤷♂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Thank you very much @jetersen for this pullrequest. Your Javascript skills are from very far better than mine :) and I must admit I gave up before getting to this level. |
Release drafter has me sharpening my JavaScript skills 😅 |
Also! Very important GitHub copilot was very good at suggesting once it had context. I don't know what I would do without it. 😅 |
This updatecli replacepattern looks like it foobars: |
spec: | ||
file: tests/main.test.js | ||
matchPattern: const version = '(.*)' | ||
replaceWith: const version = '{{ source "updatecli" }}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should have been replacepattern
reduces branching and cleans up the nested if statements and for loop.
Aligns the various logging between
linux
,windows
anddarwin
brances.hook prettier into eslint.
align trailing comma style in prettier to existing
updatecliPackages
array in index.jsremovedCould potentially also remove thecore.addPath
inupdatecliDownload
as it is called inupdatecliVersion
.tool.find
andcore.addPath
inupdatecliVersion
and keepcore.addPath
inupdatecliDownload
- Doneuse top level await which node16 does support.