-
-
Notifications
You must be signed in to change notification settings - Fork 531
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
refactor: use listr2 for the package, make and publish commands #3043
Conversation
9beb849
to
c14bbe2
Compare
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.
Going to pull this down and test, but just two initial comments:
// This logic allows developers working on forge itself to easily init | ||
// a local template and have it use their local plugins / core / cli packages |
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.
This is awesome! Any specific command devs can use to do this - is it LINK_FORGE_DEPENDENCIES_ON_INIT=true
or running link:prepare
? Just wondering if this is something we can document for future use.
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.
You run link:prepare
and then when you call electron-forge-init.js
manually you have this env var set so it links correctly
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.
Pulled this PR down and tested it a bit on Mac, it looks good to me!
This is a bit of a chonky PR but I did always say
package
was gonna be the nightmarish one.The reason this is so funky is we run
package
fromelectron-packager
and want to show internal progress without packager being away of listr. This results in thesignal
pattern I implemented (which honestly isn't that bad now it's been cleaned up a bit).The point is this output looks so dope now:
Screen.Recording.2022-11-02.at.1.14.28.AM.mov
This PR also adds support for some important things like make it possible:
The last remaining usage of
async-ora
is theimport
command which should probably be a separate PR, this one is already too chonky.