Skip to content

Conversation

@Empty2k12
Copy link
Collaborator

No description provided.

.into_iter()
.map(|(field, value)| format!("{field}={value}", field = field, value = value))
.collect::<Vec<String>>()
.join(",");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here, remove the collect


impl InfluxDbClient {
pub fn ping(&self) -> impl Future<Item = (String, String), Error = ()> {
Client::new()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you creating a new Client everytime?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How would you solve this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well, generally I prefer to have the Client be stored in a variable of the associated type and give the user a way to specify their own Client if they prefer. This might also be helpful when trying to mock http calls

}

// fixme: double space
// fixme: quoting / escaping of long strings
Copy link
Collaborator

Choose a reason for hiding this comment

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

Don't add fixme in tests. If the code doesn't do what you want, the test should fail. That's the whole idea about testing.

@Empty2k12 Empty2k12 merged commit 82b1374 into master May 31, 2019
@Empty2k12 Empty2k12 deleted the initial-commit branch May 31, 2019 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants