dutil: Add SleepWithContext

[LukeShu]

I saw a Medium post today about memory leaks when using time.After(), and thought "oh no, I/we use time.After all over the place as a cancel-able time.Sleep, I'd better figure out how to do it the right way, and put that somewhere shared!"

[rhs]

Should this be in the dtime package since elsewhere we extend stdlib packages by prefixing with a d, e.g. dexec? Also, I vaguely remember a rant somewhere about how names like util should be a last resort, and I weakly agree with the sentiment. Actually, more precisely I strongly agree with it in the long term, "util" should not be allowed to grow into a purposeless mess, but I'm fine with util as a waypoint for things to rest until they find a better home. ;-)

[LukeShu]

Counterpoint: io/ioutil, net/http/httputil.

dexec is its own thing because it's big-ish, both in size-of-code, and in that it's a drop-in replacement for a whole stdlib package. dtime wouldn't be a full-on replacement for time, it would just have that one function in it. dutil already has PanicToError and (ListenAnd)?ServeHTTP(S)?WithContext; it's small individual utilities; if split in to separate packages it would more-or-less be 1 function per package. I guess that's what I think the rule-of-thumb should be for if "util" is OK, or if it should be in a new descriptively-named package: are there going to be other things like this that will go with it, or is it a 1-off function? I don't think that there are going to be any new time-related functions. I don't think that there are going to be any new panic-related functions. I don't think that there are going to be any more HTTP-related functions.

PS: "helpers.go" isn't better than "util.go" ;-)

[rhs]

I don't think that's a good rule of thumb since it requires the user of an API to have knowledge about how large the implementation is, and whether the API will expand in the future in order to figure out what something is named. From a user's perspective, dexec and the hypothetical dtime are actually pretty much the same size:

dexec.CommandContext(ctx, ...) vs dtime.SleepWithContext(ctx, ...)

These are both also cases where we would prefer people to use the dstar version rather than stdlib version. It would be nice to be able to say if a d<foo> library exists, you should read/understand/seriously-consider-using before defaulting to the <foo> library. With your proposed rule of thumb, that becomes a much more convoluted thing to describe.

Also, I think your counterpoint is not actually a counterpoint. A counterpoint would be a util package somewhere that mixes up utilities for io and http, they are just using a suffix of util rather than a prefix of d.

Finally, on your "helpers.go" point, I agree it's not great. Naming is hard, I specifically use it for things where figuring out a good name/place has impeded progress enough that I want to get unblocked. This is an example of what I alluded to in my initial post about using a generic place for things while they wait for something better. I think in this case we already have something better.

[LukeShu]

The rule of thumb isn't so much about size-of-code as it is about size-of-API. Sure, dexec.CommandContext is just about the only thing you'll be calling off of dexec directly, but the returned *dexec.Cmd has its own big set of methods, its own contracts about returning *dexec.Error or *dexec.ExitError, and so on.

Additionally, dexec is a full drop-in replacement for os/exec (sans the context-less variant being missing). It should fully replace your use of os/exec and you should never import os/exec. That wouldn't be true of dtime. I don't see wrapping the entire time package in the future, and I don't really see any more time-related additions in the future (except I guess maybe I could imagine implementing Context-cancelable variants of *time.Ticker and *time.Timer, but I'm not hot on either of those).

The other packages in dlib (dcontext, dexec, dgroup, dlog), those are substantial building-blocks that help build an application a certain way (I'll admit, that statement is a touch dubious for dexec). dutil is... utility functions; none of them change how you structure your code, they're just little helpers for things that pop up all over the place. To me it's like left-pad; sure let's put it somewhere shared so we don't need to keep re-inventing it, but at the same time let's not make it something bigger than it is. A package with just 1 function in it is a little silly (unless that function is doing a lot, or it really wants to be its own package for import-cylce-hack reasons).

(I'll also admit that of the things in dutil, I think its the HTTP stuff that has the biggest claim to deserving a separate dhttp package; (1) the API is larger, having grown 4 variants of the same core function, and (2) it simplifies your launcher code in a lot of the same way that dexec does.)

[rhs]

Even if the rule of thumb is just about the size of the API it still requires a lot more thought for a user to apply. When I'm writing code that invokes a command, I don't start with the entire dexec API swapped into my brain, especially if I've never heard of it. I reach for the first function I need, which is just dexec.CommandContext. Perhaps later on when I want to dig into the details of error codes, I might care about dexec.ExitError, or I might never care about it. So at the moment in time when I need to apply that simple rule of "is there a dstar" thing I should look at, it really is much closer to just dexec.CommandContext vs dtime.SleepWithContext.

I'm a little confused by the nature of your pushback. I've given you a rationale for why I think the change I've suggested is better/easier for users. You've replied with a rule of thumb that retroactively allows your initial choice, but you haven't explained why that initial choice is better/easier for users, or at least if you have I haven't understood how.