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

fix(app): fix various install and version issues #14926

Merged
merged 5 commits into from
Apr 16, 2024
Merged

Conversation

sfoster1
Copy link
Member

@sfoster1 sfoster1 commented Apr 16, 2024

Fixes various ongoing issues building versions into the system. This is a follow-on to #14844 (61b1371)

  • vite define config the way we do it does not hang the defined values off of props of global explicitly (or maybe us injecting 'globalThis' into global breaks it) so use them as true globals, altering the way they're accessed and the way they're declared in the typings.
  • i guess you don't actually have to do type imports in the top level typings? removing the type import of the ipc bridge in the app-shell and app-shell-odd global.d.ts fixed that issue. don't know why
  • the ESM import for the script that updates the releases.json was wrong which breaks some update stuff

Testing

This is a bit annoying to test.

You must test this on a compiled app package. You cannot test this on a dev build.
On a compiled app package,

  • the version should display in the settings tab of the app
  • you shouldn't have warnings about include on undefined in your app logs
  • you should get robot update prompts when you use an internal-release build and connect to a robot running 1.3 or previous; you should get robot update prompts when you use a release build and connect to a robot running 7.2.1 or previous (note: couldn't test this in time but the rest of it works)
  • the help menu should have a bugs url that works (the "report an issue" button; it should pop a browser tab)
  • the help menu should say "View Opentrons App Logs" or "View Opentrons OT-3 App Logs" as the variant demands
    • it should NOT say "View App Logs". that means the package name wasn't properly interpolated.

I haven't dev-tested the second part because on my home setup making a full compiled app package is broken for some reason, and you can't actually run the node side in dev
This once more,
Closes EXEC-385
Closes RQA-2579

@sfoster1 sfoster1 requested a review from a team as a code owner April 16, 2024 20:31
@sfoster1 sfoster1 requested review from koji and removed request for a team April 16, 2024 20:31
@sfoster1 sfoster1 marked this pull request as draft April 16, 2024 20:31
@sfoster1 sfoster1 marked this pull request as ready for review April 16, 2024 21:29
@sfoster1 sfoster1 requested a review from a team as a code owner April 16, 2024 21:29
@sfoster1 sfoster1 requested a review from mjhuff April 16, 2024 21:32
@sfoster1
Copy link
Member Author

note - the tests were cancelled because i needed a build and the branch isn't named right. here's a build of a correctly-named version of the branch (with no code changes) in which the tests succeed https://github.com/Opentrons/opentrons/actions/runs/8713134684

@sfoster1 sfoster1 merged commit 717993b into edge Apr 16, 2024
33 checks passed
@sfoster1 sfoster1 deleted the exec-385-again branch April 16, 2024 23:18
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
Fixes various ongoing issues building versions into the system. This is
a follow-on to #14844 (61b1371)

- vite `define` config the way we do it does not hang the defined values
off of props of `global` explicitly (or maybe us injecting
`'globalThis'` into `global` breaks it) so use them as true globals,
altering the way they're accessed and the way they're declared in the
typings.
- i guess you don't actually have to do type imports in the top level
typings? removing the type import of the ipc bridge in the app-shell and
app-shell-odd global.d.ts fixed that issue. don't know why
- the ESM import for the script that updates the releases.json was wrong
which breaks some update stuff

## Testing
This is a bit annoying to test.

You _must_ test this on a compiled app package. You _cannot_ test this
on a dev build.
On a compiled app package,
- [x] the version should display in the settings tab of the app
- [x] you shouldn't have warnings about `include` on undefined in your
app logs
- [ ] you should get robot update prompts when you use an
internal-release build and connect to a robot running 1.3 or previous;
you should get robot update prompts when you use a release build and
connect to a robot running 7.2.1 or previous (note: couldn't test this
in time but the rest of it works)
- [x] the help menu should have a bugs url that works (the "report an
issue" button; it should pop a browser tab)
- [x] the help menu should say "View Opentrons App Logs" or "View
Opentrons OT-3 App Logs" as the variant demands
- [x] it should NOT say "View App Logs". that means the package name
wasn't properly interpolated.

I haven't dev-tested the second part because on my home setup making a
full compiled app package is broken for some reason, and you can't
actually run the node side in dev
This once more,
Closes EXEC-385 
Closes RQA-2579
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.

None yet

2 participants