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

Move code out of Main.bs into new App class #1854

Closed
wants to merge 5 commits into from

Conversation

cewert
Copy link
Member

@cewert cewert commented Jul 23, 2024

Remake of #1319

This PR accomplishes three things:

  • Move as much code out of Main.bs as possible allowing us to write unit tests for that code in the future
  • Allows us to use roku-log(m.log) for all code in Main.bs (after m.screen.show())
    • Gives a clickable link in vscode that goes to the line that created the log output
      roku-log-screen
    • Allows us to set log output as info warn verbose etc.
  • Convert print statements found in new App class to roku-log statements

Changes

  • Create App class for everything in main after m.screen.show()
  • Move roku-log setup to JFScene so we can use it in main/App class
  • Convert print statements found in new App class to roku-log statements

Issues

Fixes #1245

@cewert cewert added the dev-improvement This improves the dev experience in some way. label Jul 23, 2024
@cewert cewert marked this pull request as ready for review July 23, 2024 17:37
@cewert cewert requested a review from a team as a code owner July 23, 2024 17:37
@cewert
Copy link
Member Author

cewert commented Jul 23, 2024

Running with no crashes and I got all the print statements from Main and the main loop converted to use roku-log.

@jellyfin-bot
Copy link
Contributor

This pull request has merge conflicts. Please resolve the conflicts so the PR can be reviewed. Thanks!

@jellyfin-bot jellyfin-bot added the merge-conflict This PR has a merge conflict label Jul 29, 2024
@jellyfin-bot
Copy link
Contributor

This pull request has been inactive for 30 days and will be automatically closed in 15 days if there is no further activity.

@jellyfin-bot jellyfin-bot added the stale This issue/PR has gone stale. label Aug 29, 2024
@jellyfin-bot
Copy link
Contributor

This pull request has been closed because it has been inactive for 45 days. You may submit a new pull request if desired.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-improvement This improves the dev experience in some way. merge-conflict This PR has a merge conflict stale This issue/PR has gone stale.
Projects
Development

Successfully merging this pull request may close these issues.

Make roku-log work with files in source/
2 participants