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

[BADGER-223][WIP] Redux-based refactor of Desktop #739

Draft
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

markspolakovs
Copy link
Collaborator

No description provided.

Copy link
Contributor

github-actions bot commented Dec 28, 2024

Fails
🚫

TODOs without issue key. Please file a Linear ticket and add the number to the comment:

  • + // TODO handle error? (Probably want some kind of global handler)
  • + await forkAPI.delay(10_000); // TODO configurable
  • + logger.error("Error updating show data", e); // TODO surface to user
  • + // TODO: feedback?
  • + // TODO: Refactor this into main-side state computed by reducer
  • + // TODO: We delete it on disk before removing it from state. In theory this is
  • + // TODO: Rewrite this as a reducer which takes the current state as an action payload
🚫

Found 1 FIXME comments. Please either remove them or convert them to TODOs (with an associated Linear ticket):

  • + disabled //FIXME
Messages
📖

Removed TODOs: BDGR-175, BDGR-67

Once merged, please close the corresponding Linear tickets.
You can also include Closes BDGR-175, BDGR-67 in the PR description to close the tickets automatically.

📖

Added TODOs: BADGER-180, BDGR-67, BDGR-170

Generated by 🚫 dangerJS against 69d4e72

desktop/src/main/devServer.ts Dismissed Show dismissed Hide dismissed
@markspolakovs markspolakovs self-assigned this Dec 28, 2024

Choose a reason for hiding this comment

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

Copilot reviewed 65 out of 80 changed files in this pull request and generated no comments.

Files not reviewed (15)
  • .mise-tasks/build/desktop: Language not supported
  • .mise-tasks/test/desktop/e2e/standalone: Language not supported
  • desktop/.gitignore: Language not supported
  • desktop/package.json: Language not supported
  • desktop/src/main/base/serverApiClient.ts: Evaluated as low risk
  • desktop/src/main/base/reduxHelpers.ts: Evaluated as low risk
  • desktop/src/main/base/serverDataState.ts: Evaluated as low risk
  • desktop/src/main/base/serverConnectionState.ts: Evaluated as low risk
  • desktop/src/main/base/integrations.ts: Evaluated as low risk
  • desktop/src/main/base/selectedShow.ts: Evaluated as low risk
  • desktop/src/common/preload.ts: Evaluated as low risk
  • desktop/src/main/base/logging.ts: Evaluated as low risk
  • desktop/e2e/standalone/obs.spec.ts: Evaluated as low risk
  • desktop/e2e/standalone/base.ts: Evaluated as low risk
  • Dangerfile.ts: Evaluated as low risk
Comments suppressed due to low confidence (2)

desktop/playwright.config.ts:74

  • The environment variable name 'ENABLE_REDUX_DEVTOOLS' should be consistent with other environment variables, which are usually in uppercase.
ENABLE_REDUX_DEVTOOLS: "true",

desktop/electron.vite.config.mjs:6

  • The removal of the sentryVitePlugin configuration might be unintentional. If Sentry is still required for error tracking, please re-add the sentryVitePlugin configuration.
const { base } = require("./vite.config.mjs");
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.

1 participant