Centralize configuration and add some new config #494

trinity-1686a · 2019-03-20T19:06:01Z

trinity-1686a 评论于

2019-03-20 19:06:01 +00:00

管理员

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] 评论于

2019-03-20 19:19:25 +00:00

(从 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 (从 github.com 迁移) 评审于 2019-03-20 19:28:55 +00:00

plume-models/src/blogs.rs

elegaanz (从 github.com 迁移) 评论于

2019-03-20 19:25:08 +00:00

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.

src/main.rs

					
				@ -115,38 +127,14 @@ Then try to restart Plume.

				    })

elegaanz (从 github.com 迁移) 评论于

2019-03-20 19:28:24 +00:00

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 评审于 2019-03-20 19:58:28 +00:00

src/main.rs

					
				@ -115,38 +127,14 @@ Then try to restart Plume.

				    })

trinity-1686a 评论于

2019-03-20 19:58:28 +00:00

作者

管理员

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 评审于 2019-03-20 19:59:25 +00:00

plume-models/src/blogs.rs

trinity-1686a 评论于

2019-03-20 19:59:25 +00:00

作者

管理员

We should probably, http does not work well anyway

trinity-1686a 评审于 2019-03-20 20:29:14 +00:00

plume-models/src/blogs.rs

trinity-1686a 评论于

2019-03-20 20:29:13 +00:00

作者

管理员

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 (从 github.com 迁移) 于 2019-03-21 09:30:28 +00:00 批准此合并请求

elegaanz (从 github.com 迁移) 留下了一条评论

Looks good. Thanks. 😊

Looks good. Thanks. :blush:

trinity-1686a 评论于

2019-03-21 09:36:56 +00:00

作者

管理员

It was incomplete >_<

elegaanz 评论于

2019-03-21 10:20:21 +00:00

(从 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 评论于

2019-03-21 10:24:16 +00:00

作者

管理员

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

khimaros (从 github.com 迁移) 评审于 2019-03-26 03:53:57 +00:00

khimaros (从 github.com 迁移) 留下了一条评论

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

trinity-1686a 评论于

2019-03-26 17:01:16 +00:00

作者

管理员

@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:

登录并参与到对话中。

无评审员

elegaanz

未选择里程碑

暂无项目

未指派成员

2 位参与者

通知

到期时间

到期日期无效或超出范围。请使用“yyyy-mm-dd”格式。

未设置到期时间。

依赖工单

没有设置依赖项。

引用：Plume/Plume#494

行数
列数