Skip to content

Split off functional contact tools into its own crate#5444

Merged
Hocuri merged 2 commits intomainfrom
hoc/split-off-contact-utils
Apr 16, 2024
Merged

Split off functional contact tools into its own crate#5444
Hocuri merged 2 commits intomainfrom
hoc/split-off-contact-utils

Conversation

@Hocuri
Copy link
Copy Markdown
Collaborator

@Hocuri Hocuri commented Apr 8, 2024

I would like to implement
#5422 in its own crate, but it will depend on some functions that are in the deltachat crate.

So, this PR extracts these functions into its own crate so that I can add #5422 into the new crate.

@Hocuri Hocuri force-pushed the hoc/split-off-contact-utils branch 4 times, most recently from b227252 to 7f363e4 Compare April 8, 2024 20:21
Comment on lines +167 to +171
[workspace.dependencies]
anyhow = "1"
once_cell = "1.18.0"
regex = "1.10"
rusqlite = { version = "0.31" }
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifying the version here lets workspace members say "I'd like the same version, please" with workspace = true, this prevents out-of-sync version numbers.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this states a preference but versions will be resolved across all crates of the workspace?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more than just a preference (if I understood the documentation correctly, which I do assume) - since it says regex = "1.10" here, writing regex = { workspace = true } in a workspace member is equivalent to writing regex = "1.10"

edition = "2021"
description = "Contact-related tools, like parsing vcards and sanitizing name and address"
license = "MPL-2.0"
# TODO maybe it should be called "deltachat-text-utils" or similar?
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not completely sure what it should be called, but deltachat-contact-utils is the idea I liked best

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deltachat-contact-tools as in the PR name is also ok and maybe sounds shorter

@Hocuri Hocuri changed the title [WIP] Split off functional contact tools into its own crate Split off functional contact tools into its own crate Apr 9, 2024
@Hocuri Hocuri requested review from iequidoo and link2xt April 9, 2024 09:22
edition = "2021"
description = "Contact-related tools, like parsing vcards and sanitizing name and address"
license = "MPL-2.0"
# TODO maybe it should be called "deltachat-text-utils" or similar?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

deltachat-contact-tools as in the PR name is also ok and maybe sounds shorter

anyhow = { workspace = true }
once_cell = { workspace = true }
regex = { workspace = true }
rusqlite = { workspace = true } # Needed in order to `impl rusqlite::types::ToSql for EmailAddress`. Could easily be put behind a feature.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it can be grepped, mybe the comment is not needed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though it's possible to just grep why you need dependencies, I think it's good practice to include the justification if it's not obvious. Not something that's important for me or anything, but since you wrote "maybe", I'm going to assume that it's also not important for you

Cargo.toml Outdated
[dependencies]
deltachat_derive = { path = "./deltachat_derive" }
deltachat-time = { path = "./deltachat-time" }
deltachat-contact-utils = { path = "./deltachat-contact-utils" }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just contact-utils = ... like for ratelimit?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The latter is a crate called ratelimit stored in the directory deltachat-ratelimit. Since deltachat-contact-utils are utils specific for deltachat, so I don't think the crate should just be called contact-utils.

Hocuri added 2 commits April 16, 2024 18:06
I would like to implement
#5422 in its own
crate, but it will depend on some functions that are in the `deltachat`
crate.

So, this PR extracts these functions into its own crate so that I can
add #5422 into
the new crate.
@Hocuri Hocuri force-pushed the hoc/split-off-contact-utils branch from 7f363e4 to 92a6560 Compare April 16, 2024 16:41
@Hocuri
Copy link
Copy Markdown
Collaborator Author

Hocuri commented Apr 16, 2024

Merging despite failing CI, since CI is failing because of #4476

@Hocuri Hocuri merged commit 5d34b22 into main Apr 16, 2024
@Hocuri Hocuri deleted the hoc/split-off-contact-utils branch April 16, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants