-
Notifications
You must be signed in to change notification settings - Fork 50
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
connectivity test app (without android) #54
Conversation
can't find activity... revert local ip start architecture overview finish 1st draft of docs remove android
8e0abd7
to
f710178
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial review. Root directory only.
Co-authored-by: Vinicius Fortuna <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review for app_desktop
, shared_backend
and the IPC system.
x/outline-connectivity-app/app_desktop/generated/wailsjs/go/main/App.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! It's very useful.
x/examples/outline-connectivity-app/.yarn/plugins/@yarnpkg/plugin-typescript.cjs
Outdated
Show resolved
Hide resolved
x/examples/outline-connectivity-app/shared_frontend/assets/fonts/jigsaw_sans/bold.woff2
Outdated
Show resolved
Hide resolved
...outline-connectivity-app/shared_frontend/pages/connectivity_test/generated/messages/index.ts
Show resolved
Hide resolved
Co-authored-by: J. Yi <[email protected]>
Co-authored-by: J. Yi <[email protected]>
Note I'm going to be closing comments that don't have action items - this is just for my own sanity. Feel free to reopen anything you disagree with! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Though some generated code is still required, it's much cleaner, and my GitHub isn't stuck any more. The code looks good, added a few small comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review for shared_backend
x/examples/outline-connectivity-app/shared_frontend/pages/connectivity_test/localize.json
Outdated
Show resolved
Hide resolved
x/examples/outline-connectivity-app/shared_frontend/pages/connectivity_test/index.ts
Show resolved
Hide resolved
x/examples/outline-connectivity-app/shared_frontend/pages/connectivity_test/types.ts
Outdated
Show resolved
Hide resolved
…ectivity_test/localize.json Co-authored-by: Vinicius Fortuna <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good. The IPC calls are a lot easier to understand, thanks for the changes.
There's a README comment that you can fix, and we should use the config library, but that can be in another PR.
Great! I just need to make sure everything's working still 😅 |
README: https://github.com/Jigsaw-Code/outline-sdk/tree/outline-connectivity-app/x/outline-connectivity-app