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

code cleanup, add unit tests, hook up eslint and prettier #100

Merged
merged 20 commits into from
Jul 22, 2022

Conversation

jetersen
Copy link
Member

@jetersen jetersen commented Jul 21, 2022

reduces branching and cleans up the nested if statements and for loop.

Aligns the various logging between linux, windows and darwin brances.

hook prettier into eslint.

align trailing comma style in prettier to existing updatecliPackages array in index.js

removed core.addPath in updatecliDownload as it is called in updatecliVersion. Could potentially also remove the tool.find and core.addPath in updatecliVersion and keep core.addPath in updatecliDownload - Done

use top level await which node16 does support.

src/main.js Show resolved Hide resolved
package.json Show resolved Hide resolved
tests/main.test.js Show resolved Hide resolved
tests/main.test.js Show resolved Hide resolved
@jetersen
Copy link
Member Author

prefer squash :)

Potentially pr title could be better 🤷‍♂️

lemeurherve
lemeurherve previously approved these changes Jul 21, 2022
Copy link
Member

@lemeurherve lemeurherve left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

tests/main.test.js Show resolved Hide resolved
@lemeurherve lemeurherve requested review from olblak and dduportal July 21, 2022 13:13
@jetersen jetersen changed the title cleanup logic and hook up eslint and prettier code cleanup, add unit tests, hook up eslint and prettier Jul 21, 2022
@olblak
Copy link
Member

olblak commented Jul 22, 2022

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.

@olblak olblak merged commit 0458da2 into updatecli:v2 Jul 22, 2022
@jetersen
Copy link
Member Author

Release drafter has me sharpening my JavaScript skills 😅

@jetersen jetersen deleted the cleanupLogic branch July 22, 2022 17:43
@jetersen
Copy link
Member Author

Also! Very important GitHub copilot was very good at suggesting once it had context. I don't know what I would do without it. 😅

@jetersen
Copy link
Member Author

jetersen commented Jul 23, 2022

spec:
file: tests/main.test.js
matchPattern: const version = '(.*)'
replaceWith: const version = '{{ source "updatecli" }}'
Copy link
Member Author

Choose a reason for hiding this comment

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

should have been replacepattern

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