njsmith / posy

287 stars 17 forks source link

Loop returns first item, doesn't actually loop #15

Closed rockstar closed 1 year ago

rockstar commented 1 year ago

While trying to address clippy::never_loop, I found this code.

https://github.com/njsmith/posy/blob/70a00109a0a3d88a307897ff0f5d3bcd32262a8f/src/package_db/package_db.rs#L185-L191

This only ever operates on the first item in the list, sticks it in the cache, and returns it. I think this is a legitimate bug, but I can't entirely be sure. Like, I'm not clear on what the loop should do. It looks like we're trying to not hit the network if we don't need to, so I get the need for two loops, but I don't know what would cause the loop to continue. I'm happy to implement whatever the guidance it, but it's just not clear to me.

njsmith commented 1 year ago

Oh huh. I think it's actually correct, though it would probably be clearer to write as if let Some(ai) = matching().first() or similar?

On Mon, Jan 30, 2023 at 9:33 AM Paul Hummer @.***> wrote:

While trying to address clippy::never_loop, I found this code.

https://github.com/njsmith/posy/blob/70a00109a0a3d88a307897ff0f5d3bcd32262a8f/src/package_db/package_db.rs#L185-L191

This only ever operates on the first item in the list, sticks it in the cache, and returns it. I think this is a legitimate bug, but I can't entirely be sure. Like, I'm not clear on what the loop should do. It looks like we're trying to not hit the network if we don't need to, so I get the need for two loops, but I don't know what would cause the loop to continue. I'm happy to implement whatever the guidance it, but it's just not clear to me.

— Reply to this email directly, view it on GitHub https://github.com/njsmith/posy/issues/15, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEU42ACHUZY44XQFTV6N4LWU73NNANCNFSM6AAAAAAULNZPL4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- Nathaniel J. Smith -- https://vorpus.org http://vorpus.org