Is this equality comparison correct for checking if the current publish target is publishLocal or should something else be done?

A more idiomatic way would be to pass the corresponding skip key to publishTask directly and then call it with

    publish := publishTask(publishConfiguration, publish / skip).value,
    publishLocal := publishTask(publishLocalConfiguration, publishLocal / skip).value,

Ah yes that would be nicer, lets see what response I get from sbt maintainer to see if I am on write track and if so I can clean it up later

So I just had a quick look at this and I am not sure if this is possible without breaking binary compatibility (which I think is a concern as they appear to be public functions?)

The first issue is that if I change publishTask to `

def publishTask(
    config: TaskKey[PublishConfiguration],
    skipPublish: TaskKey[Boolean]
)

Then this will conflict with an already existing deprecated method (i.e.

@deprecated("Use variant without delivery key", "1.1.1")
def publishTask(
    config: TaskKey[PublishConfiguration],
    deliverKey: TaskKey[_],
): Initialize[Task[Unit]] =
  publishTask(config)

Both methods will have the same type after erasure.

Instead you could define

def publishTask(
    config: TaskKey[PublishConfiguration],
    skipPublish: Boolean
)

To deal with the erasure problem but on the assumption we are dealing with public functions, such a function is out of place in terms of API design. I could make this private, but I would also have to do some more additional changes to the call site since we are not dealing with a TaskKey anymore (naively doing publishTask(publishConfiguration, publish / skip).value doesn't work because the macro cannot expand the .value at that spot).

If I can get rid of the deprecated publishTask function then this change makes perfect sense. @eed3si9n What do you think?

I don't think we can remove methods during 1.x series.

Okay so you fine leaving the implementation it as is?

Okay so you fine leaving the implementation it as is?

Yea.