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

feat: deploy from Creator Hub #279

Merged
merged 22 commits into from
Nov 11, 2024
Merged

feat: deploy from Creator Hub #279

merged 22 commits into from
Nov 11, 2024

Conversation

cazala
Copy link
Contributor

@cazala cazala commented Oct 30, 2024

This PR removes the use of the linker-dapp and uses a custom UI from the within the Creator Hub

publish.flow.mp4

Copy link

Test this pull request on windows-latest

Download the correct version for your architecture:

win-x64

Copy link

github-actions bot commented Oct 30, 2024

Test this pull request on macos-latest

Download the correct version for your architecture:

mac-arm64
mac-x64

Click here if you don't know which version to download

For running this unsigned version of the app, you will need to run the xattr command on it:

  1. Extract the app from the downloaded .dmg file (double-click it)
  2. Place the extracted app anywhere you like in your file system
  3. Open a terminal on the directory where the app is
  4. Run xattr -c app-name, replacing "app-name" for the actual name of the app
  5. Double-click the app ✅

@cazala cazala marked this pull request as ready for review November 8, 2024 20:18
@cazala cazala requested a review from nicoecheza November 8, 2024 20:18
Comment on lines 297 to 302
function handleStream(stream: NodeJS.ReadableStream, type: StreamType) {
stream!.on('data', (data: Buffer) => handleData(data, matchers, type));
}

handleStream(forked.stdout!);
handleStream(forked.stderr!);
handleStream(forked.stdout!, 'stdout');
handleStream(forked.stderr!, 'stderr');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can remove this since it's duplicated with the event handlers at the top.
Also I think that having multiple data events for the same stream doesn't work properly since the data will be handled by one of them and the others will receive an empty flow of data. (not sure about the last one tho)

Comment on lines 22 to 23
setStep(prev || 'initial');
setHistory(history => (history.length > 0 ? history.slice(0, -1) : []));
Copy link
Contributor

Choose a reason for hiding this comment

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

we could probably avoid [step, setStep] by only using setHistory

we push the steps to the history array => we use the last entry on the array as the step to render. If there are no entries in the array, we always render initial. This way we avoid the double setStep, setHistory

@cazala cazala enabled auto-merge (squash) November 11, 2024 14:22
@cazala cazala merged commit 67f5c45 into main Nov 11, 2024
10 checks passed
@cazala cazala deleted the feat/linker branch November 11, 2024 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants