Streamline adding new URL types #122

Merged
wojtek merged 7 commits from 117---streamline-adding-new-url-types into main 2024-02-09 18:41:21 +01:00
Owner

Closes #117

Closes #117
wojtek added 1 commit 2024-02-08 23:22:07 +01:00
Replace hard-coded URLs with properties
Some checks failed
Cargo CI / Build and Test (pull_request) Successful in 2m23s
Cargo CI / Lint (pull_request) Failing after 39s
48119e80a5
wojtek added 1 commit 2024-02-09 07:54:05 +01:00
Update display
Some checks failed
Cargo CI / Build and Test (pull_request) Successful in 2m21s
Cargo CI / Lint (pull_request) Failing after 40s
46c61ca078
wojtek added 1 commit 2024-02-09 07:56:42 +01:00
Lints
All checks were successful
Cargo CI / Build and Test (pull_request) Successful in 1m2s
Cargo CI / Lint (pull_request) Successful in 43s
e90b541af6
wojtek reviewed 2024-02-09 10:52:11 +01:00
wojtek left a comment
Author
Owner

Also remove tmp.json.

Also remove `tmp.json`.
@ -21,2 +21,3 @@
pub sort: Option<ArtistId>,
pub properties: ArtistProperties,
pub musicbrainz: Option<MusicBrainz>,
#[serde(serialize_with = "ordered_map")]
Author
Owner

This couples the on-disk database serialisation with the in-memory abstract representation. To be fair, it was always there under the subtle guise of Serialize/Deserialize. Think whether to create the split here, or in a separate issue following this one.

This couples the on-disk database serialisation with the in-memory abstract representation. To be fair, it was always there under the subtle guise of `Serialize`/`Deserialize`. Think whether to create the split here, or in a separate issue following this one.
wojtek marked this conversation as resolved
@ -191,2 +120,2 @@
container.retain(|url| !urls.contains(url));
Ok(())
pub fn remove_from_property<S: AsRef<str>>(&mut self, property: S, values: Vec<S>) {
let container = self.properties.get_mut(property.as_ref()).map(|container| {
Author
Owner

Split map from the get_mut. Will be more legible and the container variable is used later anyway.

Split map from the get_mut. Will be more legible and the container variable is used later anyway.
wojtek marked this conversation as resolved
@ -193,0 +122,4 @@
container.retain(|val| !values.iter().any(|x| x.as_ref() == val));
container
});
if let Some(container) = container {
Author
Owner

When addressing the comment above, move the retain operation to be inside this if-let.

When addressing the comment above, move the retain operation to be inside this if-let.
wojtek marked this conversation as resolved
@ -20,0 +22,4 @@
}
}
impl<K: Hash + PartialEq + Eq, V: Merge> Merge for HashMap<K, V> {
Author
Owner

Is it possible to have a blanket merge with specialisations for ones that implement merge?

Is it possible to have a blanket merge with specialisations for ones that implement merge?
wojtek marked this conversation as resolved
wojtek added 1 commit 2024-02-09 18:23:12 +01:00
Do not for deterministic data file saving
All checks were successful
Cargo CI / Build and Test (pull_request) Successful in 1m3s
Cargo CI / Lint (pull_request) Successful in 43s
f1b4f8747d
wojtek added 2 commits 2024-02-09 18:33:50 +01:00
Streamline some implementations
All checks were successful
Cargo CI / Build and Test (pull_request) Successful in 1m2s
Cargo CI / Lint (pull_request) Successful in 42s
40bcd6b29d
wojtek added 1 commit 2024-02-09 18:38:35 +01:00
Don't be too broad with trait impl
All checks were successful
Cargo CI / Build and Test (pull_request) Successful in 1m1s
Cargo CI / Lint (pull_request) Successful in 43s
5e14fe0ac6
wojtek changed title from WIP: Streamline adding new URL types to Streamline adding new URL types 2024-02-09 18:41:14 +01:00
wojtek merged commit c2506657c3 into main 2024-02-09 18:41:21 +01:00
wojtek deleted branch 117---streamline-adding-new-url-types 2024-02-09 18:41:21 +01:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: wojtek/musichoard#122
No description provided.