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: Stop Looking for updates #178

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ArthurLobopro
Copy link
Contributor

This implements the feature request #65 and adds a manual workflow dispatch, so anyone that forks the repo can run the tests locally in the fork

@ArthurLobopro ArthurLobopro requested a review from a team as a code owner February 14, 2025 09:53
@@ -7,6 +7,7 @@ on:
schedule:
- cron: '0 22 * * 3'
workflow_call:
workflow_dispatch:
Copy link
Member

Choose a reason for hiding this comment

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

so anyone that forks the repo can run the tests locally in the fork

Could you just have this in your fork and avoid upstreaming it? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed upstream this to run the tests easier locally here. This grants me the power to run anytime the tests, I only use Linux and my hardware can't run VMs. So the only way I can test it in other platforms is using Github Actions. If you want I can remove it, but only when the other needed changes were done

src/index.ts Outdated
// check for bad input early, so it will be logged during development
const safeOpts = validateInput(opts);
export async function updateElectronApp(opts: IUpdateElectronAppOptions = {}): Promise<IUpdater> {
return new Promise((resolve, reject) => {
Copy link
Member

Choose a reason for hiding this comment

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

question: what's the intent around this Promise wrapping? From what I see, we're just manually wrapping the sync code in the Promise in this function, and initUpdater is still synchronous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified the initUpdater to return an updater controller. But when I call updateElectronApp and the app is not ready I can't return this controller because initUpdater is called inside of a callback. So I used resolve from Promise to avoid this problem and return the updater controller

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.

2 participants