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

Add launch on login feature #5244

Merged
merged 1 commit into from
May 11, 2021
Merged

Add launch on login feature #5244

merged 1 commit into from
May 11, 2021

Conversation

Xaeroxe
Copy link
Contributor

@Xaeroxe Xaeroxe commented May 11, 2021

First time contributor checklist:

Contributor checklist:

Description

Linting of dependencies doesn't succeed. No dependencies were changed in this PR, so this must be a fairly recent problem.

Fixes #1653 and adds an option to the settings menu that launches Signal on startup.

No new unit tests were added. This has been tested on Windows 10. Other operating systems are assumed to work because the feature is based on this electron API which the author assumes is well tested.

@EvanHahn-Signal EvanHahn-Signal self-requested a review May 11, 2021 18:46
@hiqua
Copy link
Contributor

hiqua commented May 11, 2021

@Xaeroxe regarding linting, the problem is that there are exceptions for certain lines that don't apply anymore with new code, so these lines have to be changed.

For me so far, the simplest way to do this has been to just remove the whole exception file, then run yarn ready and copy-paste the "questionable lines" output as the exception file.

You might need to run ts/util/lint/sort_exceptions.ts after that to have a similar order.

After that you should have a somewhat minimal diff which should be small enough to inspect manually.

The exception file is: ts/util/lint/exceptions.json

Copy link
Contributor

@EvanHahn-Signal EvanHahn-Signal left a comment

Choose a reason for hiding this comment

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

This change looks great! I'm going to merge it into a temporary branch, make a couple of small changes (including fixing the errors in exceptions.json), and get this released.

@EvanHahn-Signal EvanHahn-Signal changed the base branch from development to launch-on-login May 11, 2021 20:34
@EvanHahn-Signal EvanHahn-Signal merged commit f3dffbc into signalapp:launch-on-login May 11, 2021
@EvanHahn-Signal
Copy link
Contributor

We'll plan to release this in 5.3.0 and possibly sooner.

@Xaeroxe Xaeroxe deleted the launch-on-login branch May 11, 2021 20:36
@EvanHahn-Signal
Copy link
Contributor

@Xaeroxe Ubuntu users on our latest beta aren't seeing this property persisted, which blocks us from being able to take this to production. Any chance you could take a look?

(If I don't hear back soon, I'll plan to dig into it myself.)

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented May 19, 2021

This feature as implemented won't work on Linux systems per the Electron documentation. Probably we should just hide the checkbox on those systems. Getting the feature working on Linux will likely require a different approach.

@EvanHahn-Signal
Copy link
Contributor

@Xaeroxe That makes sense. Would you be able to open a PR in the next few hours to disable this checkbox for Linux users? If not, I can do it. (There's some urgency because it's blocking our ability to go to production.)

@Xaeroxe
Copy link
Contributor Author

Xaeroxe commented May 19, 2021

It'd probably be best if you took care of it as I imagine you'll be able to get it done faster

@EvanHahn-Signal
Copy link
Contributor

Okay, I'll do that. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[Wish] Option to start / boot with system startup
3 participants