On Fri, Jul 22, 2022 at 3:47 AM Peter Smith <smithpb2...@gmail.com> wrote: > > I was in favour of enum mostly because I thought the bitmask of an > earlier patch was mis-used; IMO each bit should only be for > representing something as "on/set". So a bit for > SUBSCRIPTION_REL_STATE_READY makes sense, but a bit for > SUBSCRIPTION_REL_STATE_NOT_READY seemed strange/backwards to me. YMMV. > > So using a bitmask is fine, except I thought it should be implemented > so that one of the bits is for a "NOT" modifier (IIUC this is kind of > similar to what Michael [1] suggested above?). So "Not READY" would be > (SUBSCRIPTION_REL_STATE_MOD_NOT | SUBSCRIPTION_REL_STATE_READY) >
Hmm, I think that sounds more complicated than what I expected. I suggest let's go with a simple idea of using a boolean not_ready which will decide whether to use the additional key to search. I feel we can extend it by using a bitmask or enum when we have a clear need for more states. > Also, it may be better to add the bit constants for every one of the > current states, even if you are not needing to use all of them just > yet. In fact, I thought this patch probably can implement the fully > capable common function (i.e. capable of multiple keys etc) right now, > so there will be no need to revisit it again in the future. > I don't know whether we need to go that far. Say for a year or so if we don't have such a use case arising which appears to be quite likely then one can question the need for additional defines. -- With Regards, Amit Kapila.
