Add environment variable to select storage provider #741
Conversation
Signed-off-by: Bernhard Bermeitinger <[email protected]>
…g.signal.Signal into minosimo-storage_env_variable
Signed-off-by: Bernhard Bermeitinger <[email protected]>
|
Started test build 149984 |
|
Build 149984 successful |
* Add environment variable to select storage provider * Simplify password store env variable * Add password store description to readme * Validate the password store and print an info when using basic Signed-off-by: Bernhard Bermeitinger <[email protected]> * Show warning about (lack of) encryption * Reword the notification Signed-off-by: Bernhard Bermeitinger <[email protected]> --------- Signed-off-by: Bernhard Bermeitinger <[email protected]> Co-authored-by: Simon <[email protected]> Co-authored-by: bbhtt <[email protected]>
|
The warning currently shows on every other launch. |
|
Do we agree it would be better to show the warning only on new installs? Checking if config.json exists should be sufficient. |
* signal-desktop: Migrate from `signal.desktop` (#739) Ideally, upstream will also migrate to `org.signal.Signal.desktop`. This fixes task manager icons on KDE Plasma 6.1.5. AppStream gained `<provides><id>…` per a recommendation from <https://blogs.gnome.org/hughsie/2019/04/23/remember-the-extra-metadata-if-you-change-a-desktop-id/>. * Add environment variable to select storage provider (#741) * Add environment variable to select storage provider * Simplify password store env variable * Add password store description to readme * Validate the password store and print an info when using basic Signed-off-by: Bernhard Bermeitinger <[email protected]> * Show warning about (lack of) encryption * Reword the notification Signed-off-by: Bernhard Bermeitinger <[email protected]> --------- Signed-off-by: Bernhard Bermeitinger <[email protected]> Co-authored-by: Simon <[email protected]> Co-authored-by: bbhtt <[email protected]> --------- Signed-off-by: Bernhard Bermeitinger <[email protected]> Co-authored-by: bb010g <[email protected]> Co-authored-by: Simon <[email protected]> Co-authored-by: bbhtt <[email protected]>
| esac | ||
|
|
||
| if [[ "${SIGNAL_PASSWORD_STORE}" == "basic" ]]; then | ||
| if [[ -f "${XDG_CACHE_HOME}"/warning-shown ]]; then |
There was a problem hiding this comment.
This condition won't work, this will always be true if you stay on basic. After the first time as the file exists this removes it on the next launch and on the next one even if you are on basic and have seen the warning it will show up again. It becomes a cyclic thing.
There was a problem hiding this comment.
You also have to take the rm part out of the first if password store == basic condition.
If someone switches from basic to encrypted and back to basic again the warning should be shown.
rm is piped to true so it doesn't matter if the file previously didn't exist.
|
Can we please revert the breaking change while we work on a proper fix? |
|
This should effectively revert it, what do you want now? The only issue is that the warning is shown on every other launch instead of once. |
|
This MR is not ready, I'm simply suggesting reverting #729 immediately as a temporary fix. I don't see a downside in doing so. |
|
There is no reason to revert #729 now. The environment variable already achieves what the revert would do. The issue about showing the warning only once needs to be fixed. |
|
Could you look at #744 and let's continue the discussion there. |
Supersedes #738