Centralize configuration and add some new config #494

Unito
Plume_migration_agent ha unito 6 commit da config a master 5 anni fa
trinity-1686a 5 anni fa ha commentato
Proprietario

Ideally, if someone could review the idea in this comment, I'd like to add the implementation to this pr too

Ideally, if someone could review the idea in [this comment](https://github.com/Plume-org/Plume/issues/273#issuecomment-474982184), I'd like to add the implementation to this pr too
codecov[bot] 5 anni fa ha commentato (Migrato da github.com)

Codecov Report

Merging #494 into master will increase coverage by 0.05%.
The diff coverage is 31.74%.

@@            Coverage Diff             @@
##           master     #494      +/-   ##
==========================================
+ Coverage   26.67%   26.72%   +0.05%     
==========================================
  Files          64       65       +1     
  Lines        8713     8741      +28     
==========================================
+ Hits         2324     2336      +12     
- Misses       6389     6405      +16
# [Codecov](https://codecov.io/gh/Plume-org/Plume/pull/494?src=pr&el=h1) Report > Merging [#494](https://codecov.io/gh/Plume-org/Plume/pull/494?src=pr&el=desc) into [master](https://codecov.io/gh/Plume-org/Plume/commit/b945d1f602aba50d2cfdc77eba0742f6e707632b?src=pr&el=desc) will **increase** coverage by `0.05%`. > The diff coverage is `31.74%`. ```diff @@ Coverage Diff @@ ## master #494 +/- ## ========================================== + Coverage 26.67% 26.72% +0.05% ========================================== Files 64 65 +1 Lines 8713 8741 +28 ========================================== + Hits 2324 2336 +12 - Misses 6389 6405 +16 ```
elegaanz (Migrato da github.com) revisionato 5 anni fa
elegaanz (Migrato da github.com) 5 anni fa ha commentato

Not really related, but I'm thinking about it because I'm seeing it: shouldn't we drop this setting and force people to set up HTTPS? I introduced it to make it easier to develop locally, but now this requires HTTPS too because of CSRF.

Not really related, but I'm thinking about it because I'm seeing it: shouldn't we drop this setting and force people to set up HTTPS? I introduced it to make it easier to develop locally, but now this requires HTTPS too because of CSRF.
@ -115,38 +127,14 @@ Then try to restart Plume.
})
elegaanz (Migrato da github.com) 5 anni fa ha commentato

Isn't there a way to use the Config struct from rocket directly in our config module instead of RocketConfig?

Isn't there a way to use the `Config` struct from rocket directly in our `config` module instead of `RocketConfig`?
trinity-1686a revisionato 5 anni fa
@ -115,38 +127,14 @@ Then try to restart Plume.
})
trinity-1686a 5 anni fa ha commentato
Autore
Proprietario

RocketConfig could become a Result<rocket::Config> (Result to denote when there was an error creating the Config, or when there is no secret key). I through of it at some point, but did not remember what I wanted to do when I was about to commit

`RocketConfig` could become a `Result<rocket::Config>` (Result to denote when there was an error creating the Config, or when there is no secret key). I through of it at some point, but did not remember what I wanted to do when I was about to commit
trinity-1686a revisionato 5 anni fa
trinity-1686a 5 anni fa ha commentato
Autore
Proprietario

We should probably, http does not work well anyway

We should probably, http does not work well anyway
trinity-1686a revisionato 5 anni fa
trinity-1686a 5 anni fa ha commentato
Autore
Proprietario

As someone on Matrix said they were using Plume on a Tor Hidden Service, I tried to see if this won't cause any CSRF issues, and apparently Tor Browser consider hidden service to be secure even without https, that's nice

As someone on Matrix said they were using Plume on a Tor Hidden Service, I tried to see if this won't cause any CSRF issues, and apparently Tor Browser consider hidden service to be secure even without https, that's nice
elegaanz (Migrato da github.com) hanno approvato queste modifiche 5 anni fa
elegaanz (Migrato da github.com) lascia un commento

Looks good. Thanks. 😊

Looks good. Thanks. :blush:
trinity-1686a 5 anni fa ha commentato
Autore
Proprietario

It was incomplete >_<

It was incomplete >_<
elegaanz 5 anni fa ha commentato (Migrato da github.com)

Sorry… I didn't saw you added the "Incomplete" label… 🤦‍♀️

Sorry… I didn't saw you added the "Incomplete" label… :woman_facepalming:
trinity-1686a 5 anni fa ha commentato
Autore
Proprietario

No worries, I made another pr with what I wanted to add

No worries, I made another pr with what I wanted to add
khimaros (Migrato da github.com) revisionato 5 anni fa
khimaros (Migrato da github.com) lascia un commento

Would be rad if 'plm' honored this config as well. Fortunately, easy to workaround with command line flag.

Would be rad if 'plm' honored this config as well. Fortunately, easy to workaround with command line flag.
trinity-1686a 5 anni fa ha commentato
Autore
Proprietario

@khimaros you can open an issue with what you think would be a better comportment 😉

@khimaros you can open an issue with what you think would be a better comportment :wink:

Revisori

La pull request è stata unita come 65bb50e88f.
Puoi anche visualizzare le istruzioni da riga di comando.

Passo 1:

Dal repository del tuo progetto, fai il check out di un nuovo branch e verifica le modifiche.
git checkout -b config master
git pull origin config

Passo 2:

Fai il merge delle modifiche e aggiorna su Forgejo.
git checkout master
git merge --no-ff config
git push origin master
Effettua l'accesso per partecipare alla conversazione.
Nessun revisore
Nessuna milestone
Nessuna assegnatario
2 Partecipanti
Notifiche
Data di scadenza
La data di scadenza non è valida o fuori intervallo. Si prega di utilizzare il formato 'aaaa-mm-dd'.

Nessuna data di scadenza impostata.

Dipendenze

Nessuna dipendenza impostata.

Riferimento: Plume/Plume#494
Caricamento…
Non ci sono ancora contenuti.