Skip to content
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

A mixed bag of fixes :) #666

Merged
merged 1 commit into from
Oct 14, 2023
Merged

A mixed bag of fixes :) #666

merged 1 commit into from
Oct 14, 2023

Conversation

nyurik
Copy link
Contributor

@nyurik nyurik commented Oct 13, 2023

  • Fixed Build badge link to CI
  • A few grammar & spelling mistakes
  • inline format args where appropriate - makes it a bit easier to read and follow
  • Removed un-needed borrow (per clippy suggestion)
  • Replaced .rev().next() with next_back() (per clippy)
  • Ran cargo fmt to keep things a bit tidier

@nyurik
Copy link
Contributor Author

nyurik commented Oct 13, 2023

P.S. in all seriousness though - I was looking at some unwrap() - seems they may crash during execution (I am still investigating) - and not certain if it would make sense to change them to Results instead?

@RazrFalcon
Copy link
Collaborator

Thanks, but I don't think I'm ready to switch to inline format. Let's keep it as is.

@nyurik
Copy link
Contributor Author

nyurik commented Oct 13, 2023

@RazrFalcon any objections to the other changes?

@RazrFalcon
Copy link
Collaborator

Hard to differentiate them, but seems ok.

I just don't like the fact that inline format doesn't support expressions. Feels half-backed.

* Fixed Build badge link to CI
* A few grammar & spelling mistakes
* Removed un-needed borrow (per clippy suggestion)
* Replaced `.rev().next()` with `next_back()` (per clippy)
* Ran `cargo fmt`
@RazrFalcon RazrFalcon merged commit dfda83e into linebender:master Oct 14, 2023
2 of 3 checks passed
@RazrFalcon
Copy link
Collaborator

Thanks. In general I do not accept code formatting and clippy fixes.

@nyurik nyurik deleted the cleanup branch October 14, 2023 17:50
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.

2 participants