Skip to content

Draft: Out-of-box Experience #41

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Draft: Out-of-box Experience #41

wants to merge 1 commit into from

Conversation

roobscoob
Copy link
Contributor

@roobscoob roobscoob commented Nov 21, 2024

Looking for feedback on the OOBE. This PR depends on #39

This PR makes the DISCORD_TOKEN env argument obsolete. The environment variable will still be respected if available, and skip the OOBE.

@roobscoob roobscoob requested a review from alii November 21, 2024 01:02
@circularsprojects
Copy link
Contributor

this looks really good so far!!!

@Bloeckchengrafik
Copy link
Contributor

Looks great so far! Just a heads-up:
GPUI provides a way to store credentials in the platform keychain, which is more secure than simple file storage in many cases.

See https://github.com/zed-industries/zed/blob/eb2c0b33dff361a268b0276f8ec8bb38811d2c5e/crates/gpui/src/app.rs#L639-L652 for details

It might be worth opening an issue for this once the PR is merged, but I think implementing it now would be a better way.

What do you think?

@roobscoob
Copy link
Contributor Author

roobscoob commented Dec 1, 2024

Not in favor of using GPUI's credential API because it's so baked around the (url, username, password) tuple. We could hypothetically get around this by doing something like ("discord://token", "roobscoob", <censored>) but then I would say that it's lack of supporting multiple (username, password) pairs per url will bite us.

@jacksongoode
Copy link

36/crates/gpui/src/platform/mac/metal_renderer.rs
  cargo:rerun-if-changed=./src/platform/mac/shaders.metal

  --- stderr
  metal shader compilation failed:
  xcrun: error: unable to find utility "metal", not a developer tool or in PATH

warning: build failed, waiting for other jobs to finish...

I don't have Xcode installed but running into this build issue on macOS.

@DovidP
Copy link
Contributor

DovidP commented Dec 3, 2024

You need xcode installed. Also this is a gpui error, not scope.

@jacksongoode
Copy link

jacksongoode commented Dec 3, 2024

Got it built, it seems that an invalid token will not fall back to login again? Is this PR accepting commits?

@circularsprojects
Copy link
Contributor

iirc we can just query "users/@me" or something similar (which serenity already has a method for)
if it's invalid we get an error code and if it's valid we get username and stuff which we can use for a welcome screen like "welcome circular!"

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.

5 participants