Skip to content

Overall code improvements #1118

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 28, 2020
Merged

Overall code improvements #1118

merged 1 commit into from
Oct 28, 2020

Conversation

martskins
Copy link
Collaborator

This PR is a bit of everything, but the overall goal is to make the code a little more readable and efficient 😄
It essentially does three things:

  • Adds the crates tracing and tracing-log: this enables us to replace all the manual logs we have in our methods with the #[tracing::instrument] macro, which will log the name of the function and the parameters it was called with. This makes the code much easier to read and uniform, and also makes the logs easier to read, as the function name and arguments are now logged on the same line and we get basically one line per function call.

  • Removes unnecessary logs: now that we have the above we no longer need to log all the messages we receive and send to the server, as we're already logging the parameters with which we call the functions anyways, so this will make the logs a lot easier to read as well.

  • Removes unnecessary clones: We had a lot of cases where we called serde_json::from_value(value.clone()) when really we could be calling T::deserialize(&value), which doesn't take ownership of value and achieves exactly the same.

@martskins martskins changed the base branch from next to dev October 28, 2020 19:53
@martskins martskins merged commit 353e9de into autozimu:dev Oct 28, 2020
martskins added a commit to martskins/LanguageClient-neovim that referenced this pull request Dec 6, 2020
martskins added a commit to martskins/LanguageClient-neovim that referenced this pull request Dec 6, 2020
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.

1 participant