jonhoo / buzz

A simple system tray application for notifying about unseen e-mail
Apache License 2.0
138 stars 12 forks source link

Restructure config #28

Closed DanielVoogsgerd closed 6 months ago

DanielVoogsgerd commented 10 months ago

As promised, a restructured version of the config as mentioned in: #6.

I tried to keep fairly consistent with the current code structure and not refactor too much. However, I think it might not be the worst idea to refactor at least the configuration parsing, as this is becoming quite unwieldy.

This however will probably suffice for the major version bump. If you like to see any changes or if you have some tips, let me know, those are always welcome.

codecov-commenter commented 10 months ago

Codecov Report

Attention: Patch coverage is 0% with 124 lines in your changes are missing coverage. Please review.

Project coverage is 0.0%. Comparing base (d932999) to head (41fe28d). Report is 7 commits behind head on main.

Additional details and impacted files | [Files](https://app.codecov.io/gh/jonhoo/buzz/pull/28?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset) | Coverage Δ | | |---|---|---| | [src/tray\_icon.rs](https://app.codecov.io/gh/jonhoo/buzz/pull/28?src=pr&el=tree&filepath=src%2Ftray_icon.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset#diff-c3JjL3RyYXlfaWNvbi5ycw==) | `0.0% <0.0%> (ø)` | | | [src/main.rs](https://app.codecov.io/gh/jonhoo/buzz/pull/28?src=pr&el=tree&filepath=src%2Fmain.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Jon+Gjengset#diff-c3JjL21haW4ucnM=) | `0.0% <0.0%> (ø)` | |
jonhoo commented 10 months ago

Oh, sorry I wasn't clear — I think we should move to #[derive(Deserialize)] for an actual type (or set of types). That should make all of this much less unwieldy :sweat_smile:

See https://docs.rs/toml/latest/toml/#deserialization-and-serialization for a reference.

DanielVoogsgerd commented 10 months ago

No worries, I tried to stay consistent with the current codebase to get the changes as small as possible, but I will gladly move the deserialization to serde.

DanielVoogsgerd commented 8 months ago

That took longer than I had hoped, but I finally found some time to do the refactor. Not my best work; there is a fair bit of cloning that can be removed by restructuring or reference counting, but I wanted to keep the changes semi in-scope. Feel free to nitpick, but I'm not sure how soon I can get around to fixing everything. Feedback is always welcome.

Thanks

DanielVoogsgerd commented 8 months ago

Thanks for the detailed feedback, almost all the suggestions have been applied, only the removal of the clones is not really possible I think (see response on review). This could possibly be solved using references, but I'm not sure if that is worth the hassle.

DanielVoogsgerd commented 8 months ago

I don't think that is correct. Where normally, we would define accounts as named tables. It has been replaced with an array of tables called account now.

That does remind me of the fact that we should update the README accordingly. I'll try to push a change later today.

Considering the support around the tray-icon has changed as well, I think a major bump is due.

One final thing to consider is that if you bump the version, do you want to continue to support both the folder and the folders configuration option. I did it like this to make it backwards compatible, but maybe it is best to drop folder right now, as people will have to adjust their configuration anyway.

jonhoo commented 8 months ago

Good call!

And yes, I think you're right we should then just have folders :+1:

DanielVoogsgerd commented 6 months ago

I'll try to push a change later today.

Yeah, that did not happen :sweat_smile:

But, I think it should be correct now. folder has been removed and README.md has been updated.

jonhoo commented 6 months ago

Perfect, thank you! Release coming shortly :+1:

jonhoo commented 6 months ago

Released as 2.0.0 :tada: