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

refactor: use listr2 for the package, make and publish commands #3043

Merged
merged 8 commits into from Nov 3, 2022

Conversation

MarshallOfSound
Copy link
Member

@MarshallOfSound MarshallOfSound commented Nov 2, 2022

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 from electron-packager and want to show internal progress without packager being away of listr. This results in the signal 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:

  • An adapter for hooks into listr tasks
  • The ability for internal hooks (no external facing change) to provide custom names to listr and receive an instance of the listr task

The last remaining usage of async-ora is the import command which should probably be a separate PR, this one is already too chonky.

@MarshallOfSound MarshallOfSound changed the title refactor: use listr2 for the package command refactor: use listr2 for the package and make command Nov 3, 2022
@electron electron deleted a comment from codecov-commenter Nov 3, 2022
@MarshallOfSound MarshallOfSound changed the title refactor: use listr2 for the package and make command refactor: use listr2 for the packag, make and publish commands Nov 3, 2022
@MarshallOfSound MarshallOfSound changed the title refactor: use listr2 for the packag, make and publish commands refactor: use listr2 for the package, make and publish commands Nov 3, 2022
Copy link
Member

@VerteDinde VerteDinde left a 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:

Comment on lines +38 to +39
// This logic allows developers working on forge itself to easily init
// a local template and have it use their local plugins / core / cli packages
Copy link
Member

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.

Copy link
Member Author

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

packages/api/core/src/api/publish.ts Outdated Show resolved Hide resolved
Copy link
Member

@VerteDinde VerteDinde left a 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!

@MarshallOfSound MarshallOfSound merged commit aca89f4 into main Nov 3, 2022
@MarshallOfSound MarshallOfSound deleted the listr-package branch November 3, 2022 23:01
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.

None yet

2 participants