Conversation
0e0391c to
7a443bb
Compare
Swiftb0y
left a comment
There was a problem hiding this comment.
Can you elaborate a bit more on the high-level principle of these Settings? What data is contained in which file, what files are exported, which are expected to be present on an external drive, etc?
Also in terms of code, I'm getting a bit confused because I regularly mix up Settings (high-level API) and Setting (binrw).
| pub mod anlz; | ||
| pub mod device; | ||
| pub mod pdb; | ||
| pub mod setting; | ||
| pub mod util; | ||
| pub(crate) mod xor; |
There was a problem hiding this comment.
Do we want to make the more internal modules private when they have a nicer API counterpart.
There was a problem hiding this comment.
Not sure. Maybe we should group the low-level stuff in a separate submodule or crate but leave it public for people who want to experiment or squeeze out maximum performance?
There was a problem hiding this comment.
@Holzhaus another layer makes sense. Basically a "core" library.
(hi gang, I've been watching the progress and though I would chime in. Hope it's not TOO weird to have me just pop into the conversation).
| #[derive(Debug, Default, PartialEq, Eq, Clone)] | ||
| pub struct Settings { |
There was a problem hiding this comment.
What do we want the Default to be? Just a struct with all Nones (which is derived) or with the actual default settings from rekordbox?
There was a problem hiding this comment.
Rekordbox' default settings probably make sense.
src/main.rs
Outdated
| let mut export = DeviceExport::default(); | ||
| export.load(path)?; | ||
| let settings = export.get_settings(); |
There was a problem hiding this comment.
IMO constructing a mutable default and then loading from a file doesn't seem very rusty to me. Why not make load a constructor? Also allows for nice chaining.
| let mut export = DeviceExport::default(); | |
| export.load(path)?; | |
| let settings = export.get_settings(); | |
| let settings = DeviceExport::load(path)?.get_settings(); |
0679936 to
8a75859
Compare
a5eaf21 to
8a44d90
Compare
This is an attempt to look into a high-level API to make it easier to read from (and later also write to) Rekordbox device exports without knowing much about the internals.
As first step, I only added support for setting files:
The CLI makes use of the new API when calling the new
list-settingssubcommand:Based on #90 and #91.