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

Improve download behavior #27

Merged
merged 6 commits into from
Nov 16, 2022
Merged

Improve download behavior #27

merged 6 commits into from
Nov 16, 2022

Conversation

trickypr
Copy link
Member

There are a few problems with download that this solves:
a) There isn't enough logging, especially with GitHub addons, to be helpful in error reports
b) Download didn't allow you to download extensions without redownloading engine/ (regression from #25)

@splatboydev because this is changing behavior, can you review this?

splatboydev and others added 3 commits November 14, 2022 19:33
Should fix #24. This ensures that it adds the gluon dir rather than a file names gluon.
The primary justification for this is adding/updating extensions.
Ideally, you should just be able to run `download` again to get
the new extensions, which the current implementation does not
allow for.
@trickypr trickypr added the bug Something isn't working label Nov 14, 2022
@splatboydev
Copy link
Contributor

I personally suggest adding some kind of force argument, but that isn't necessary.

log.info("'engine/' is empty, deleting contents...")
rmSync(ENGINE_DIR, { recursive: true })

}
Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean if it isn't empty? If firefox has already been downloaded, it will skip downloading it and go to downloading addons.

Copy link
Member

@PressJump PressJump left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@splatboydev splatboydev left a comment

Choose a reason for hiding this comment

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

LGTM

@trickypr trickypr merged commit c8484ec into main Nov 16, 2022
@trickypr trickypr deleted the correct-downloads-behavior branch November 16, 2022 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants