chore: require kwargs for utils.copy_dict()
#1871
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1871 +/- ##
=======================================
Coverage 92.51% 92.51%
=======================================
Files 78 78
Lines 4878 4878
=======================================
Hits 4513 4513
Misses 365 365
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Should we switch them everywhere? If someone uses this undocumented function outside the library I would be slightly confused :D |
Sorry I don't understand? Switch what everywhere? Maybe my description should have been: The non-keyword argument order in |
|
I understand, but since we're already making a breaking change by introducing required keyword arguments, we can also switch them to be src first, dest, second - both in the definition and when they're called. Would that make sense? |
|
I guess I don't think of it as a breaking change. But maybe that means we need to define what is considered a breaking change. What part of the API do we consider covered for it be a breaking change? We probably need to document that. I don't think an internal utility library function is part of our API. |
Sorry, I wasn't super clear there, I didn't mean something user-facing or to add to the changelog, just that the way the function can be used now has changed. So if we're already doing that, we can also correct the order and switch the args :) |
Ah! That makes total sense to me 👍 |
The non-keyword arguments were a tiny bit confusing as the destination was first and the source was second. Change the order and require key-word only arguments to ensure we don't silently break anyone.
3984328 to
7cf35b2
Compare
The non-keyword arguments were a tiny bit confusing as the destination was
first and the source was second.
Change the order and require key-word only arguments to ensure we
don't silently break anyone.