Codecov Report

Merging#1871 (7cf35b2) intomain (64d01ef) willnot change coverage.
The diff coverage is100.00%.

@@           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.

Non-keyword arguments are a tiny bit confusing as the destination is first and the source is second.

Should we switch them everywhere? If someone uses this undocumented function outside the library I would be slightly confused :D

Non-keyword arguments are a tiny bit confusing as the destination is first and the source is second.

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 inutils.copy_dict() is confusing as the destination is first and the source is second.

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.

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 :)

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 👍