chore(deps): replace internal/slices#63386
Conversation
134eedf to
a390315
Compare
|
I see |
|
@Strum355 in the PR I built on top of this one, I want to use MapFilter: https://github.com/sourcegraph/sourcegraph/pull/63387/files#diff-64165e89e585f409119585ed3d24e70ae75a24c984f00a990e1c77ecfcf7d84bR74 stdlib slices doesn't have a filter function either. I found out about this lib from devx-service (https://github.com/sourcegraph/devx-service/blob/26b3819ef2c6b9ae3d67f767a0a4ebb8cace1535/go.mod#L16), there's a lot of useful calls to it in there 😁 Got an alternative in mind out of interest? I would be likely to add filter and/or mapfilter to internal/slices if we don't want to bring in another lib. I don't really mind either way tbh! |
With an open-source package with more features. Ran: ``` sg bazel configure bazel run //:gazelle-update-repos ```
a390315 to
5dcfe64
Compare
It does but its called DeleteFunc. I wont claim Map(DeleteFunc()) is gonna be as nice to write though, so Im not particularly opposed to this |
|
Oh, I totally missed that! Thanks for approving though |
|
@craigfurman there was previous discussion on this in Slack, and there was net opposition to using this kind of helper library. https://sourcegraph.slack.com/archives/C3B3SDBMY/p1715166942685459 That was including @Strum355, so I'm honestly confused as to why he approved this PR 😅 |
I saw this library was used over in devx-service by @jhchabran, I saw it mentioned here in the context of its slices package for a MapFilter instead of combining our slices.Map with stdlib's DeleteFunc (which was the thing I specifically didnt like, having to do |
|
In hindsight I shouldve probably pushed for MapFilter to be added to our own internal/slices, I put a lot of weight on the lib having been used in devx-service in that moment 🙂 |
|
🤷 I wasn't aware of that discussion. If someone wants to revert this and update current consumers to use an alternative, that's fine 🙂 I'm not sure what the value would be in reimplementing bits of this collections library though? I think it's a pretty low-value decision in either direction, to be quite honest 😅 |
|
I should add: this is definitely a 2-way door decision! |
With an open-source package with more features.
Ran:
Between writing that little slices package, and wanting more from it in an upcoming PR, I found out this 3rd party library existed. It seems very good.
Draft until base branch merged, but this can be reviewed independently.
LMK if
chore(deps)isn't the right sort of title for the changelog, looks OK based on former PRs.Test plan
Tests should still pass.
Changelog