modpublisher icon indicating copy to clipboard operation
modpublisher copied to clipboard

Mark GitHub Release as draft if tag doesn't exist

Open MattSturgeon opened this issue 1 year ago • 7 comments

While working on #15 I came to the conclusion that createTag=false and draft=true are kinda incompatible. Or at least unintuitive.

I realised while working on https://github.com/MinecraftFreecam/Freecam/pull/195 that it'd be useful to mark a release as a draft only when the tag doesn't exist yet.

I'm not sure the best API for this. Maybe some options would become enums, but with boolean and String overloaded setters?

MattSturgeon avatar Apr 04 '24 22:04 MattSturgeon

Sorry, been busy with work stuff. Meant to answer you earlier on the PR comment.

that it'd be useful to mark a release as a draft only when the tag doesn't exist yet.

This was actually our original behavior. The release was only marked as a draft, if it didn't exist when the upload started. That way all the artifact uploads could finish, before the release was "made public"

We could either as you suggested, rename it for clarity, or, move it to an enum/boolean

hypherionmc avatar Apr 04 '24 22:04 hypherionmc

This was actually our original behavior. The release was only marked as a draft, if it didn't exist when the upload started. That way all the artifact uploads could finish, before the release was "made public"

Sorry for being unclear. The draft option controls what happens to the release after uploading the artifacts, not when creating the release.

So, regardless of what you set github.draft to, the release will always start off as a draft (unless the release already exists).

This issue is about keeping the release as a draft if the tag didn't exist (not release), although I suppose it may also be useful to also have the keep-as-draft-if-release-didn't-exist option too...

MattSturgeon avatar Apr 04 '24 22:04 MattSturgeon

I wonder if the best way to approach this would be a custom predicate?

We could pass a context object to a markDraft predicate with properties such as:

  • wasDraft
  • tagExists
  • releaseExists

And use the return value in releaseUpdater.draft()?

The default predicate could just be (ctx) -> false to maintain the previous behaviour.

MattSturgeon avatar Apr 04 '24 22:04 MattSturgeon

I wonder if the best way to approach this would be a custom predicate?

We could pass a context object to a markDraft predicate with properties such as:

* wasDraft

* tagExists

* releaseExists

And use the return value in releaseUpdater.draft()?

This sounds like a good compromise. To make it easier to implement, this could be tied to an Enum. That way you could pass in the enum, or a string, similar to ReleaseType, ModLoader etc

hypherionmc avatar Apr 04 '24 22:04 hypherionmc

To make it easier to implement, this could be tied to an Enum.

An enum of default predicates?

Not sure it'd make implementation easier, but it would make common use-cases simpler.

enum GHDraftPredicate {
    NEVER(ctx -> false),
    ALWAYS(ctx -> true),
    NEW_TAG(ctx -> !ctx.existingTag);
    // etc

    // Predicate field
    @Getter
    private Predicate<ReleaseContext> predicate;

    // Example factory
    static GHDraftPredicate valueOf(boolean val) {
        return val ? ALWAYS : NEVER;
    }

    // Constructor, factories, etc omitted for clarity
}

The draft option would then have overloaded setters accepting Predicate<ReleaseContext>, GHDraftPredicate or anything that can be handled by the enum's factories (string, boolean)

MattSturgeon avatar Apr 04 '24 23:04 MattSturgeon

@MattSturgeon Is this issue still on-going? Checking the PR linked, you mentioned it fixes this issue.

Just want to double check before closing it out

hypherionmc avatar Feb 01 '25 21:02 hypherionmc

I don't think this got tackled, beyond a few related issues.

I've not been very active recently on the project where I use modpublisher, and TBH I can't recall exactly what the motivation was for this issue 😅

Feel free to close as stale or not-planned, as it has been a while...

MattSturgeon avatar Feb 02 '25 03:02 MattSturgeon