search

time-rs / time

The most used Rust library for date and time handling.
https://time-rs.github.io
Apache License 2.0
1.13k stars 281 forks source link

style: Remove unnecessary commas #707

Closed sorairolake closed 1 month ago

sorairolake commented 2 months ago

Looking at other codes, it seems better to eliminate these commas. So I'm going to remove these commas.

jhpratt commented 2 months ago

This likely isn't exhaustive, as I'm pretty sure I've run across more than just these in passing. Can you be sure that it is?

sorairolake commented 2 months ago

@jhpratt This is applied to lines which match rg -F "),)". So I cannot be sure that this is exhaustive, and I don't think this likely is exhaustive.

First, I discovered a redundant comma in the following doctest:

https://github.com/time-rs/time/blob/2e68a36abc0d8cdc12638c0ed47b80cc9c07b7ab/time/src/offset_date_time.rs#L77

Next, I ran rg -F "),)" and discovered a similar redundant comma in the following line:

https://github.com/time-rs/time/blob/2e68a36abc0d8cdc12638c0ed47b80cc9c07b7ab/time/src/serde/mod.rs#L208

jhpratt commented 2 months ago

If cleanup like this is being done, I'd like to be at least reasonably confident that it is exhaustive, lest we have a number of PRs substantially similar to this.

jhpratt commented 1 month ago

@sorairolake What's the status of this?

sorairolake commented 1 month ago

@jhpratt I'm not confident that I can do cleanups like this thoroughly.

jhpratt commented 1 month ago

Closing for that reason. I'll probably end up digging through code myself with some more complicated regex in the near future.