Fix 'sync-labels' setting

[epuertat]

Closes #104

Following syntax at https://github.com/actions/toolkit/blob/master/packages/glob/README.md#recommended-action-inputs

[pje]

Thanks for the patch, @epuertat! Labeler was behaving incorrectly and your changes corrected that behavior. I took the liberty of updating to the current state of main branch, and adding tests.

What this means is we're in the weird position where Doing The Right Thing means a breaking change, since labeler will now behave differently with the same inputs out there in the wild. A load-bearing bug.

IMO this necessitates a v4 release and a big callout in the changelog.

Before

Before this change: if you provide literally any value for sync-labels, labeler would Sync Labels. (In particular, if you had sync-labels: false, labeler would actually Sync Labels. Yikes.)

After

After this change: you must provide true to get Sync Labels behavior. Providing false will NOT Sync Labels. (And any other value will be treated like false.)

[epuertat]

Thanks for taking care of the tests, @pje!

[tupui]

@pje Any ETA on merging and deploying a new version? We are considering removing the action in SciPy because the sync mechanism is annoying us. I really like the idea of this action and would like to keep it 😃

[wknapik]

Hey. Can you please merge/release this?

[hashhar]

Anything we can do to help move this forward and get this merged?

[tupui]

I am sad to say that due to this, we (SciPy) dropped this actions. Hope that this get solved at some point so we can reconsider.

[Jackenmen]

@tupui the workaround for this is fairly easy, you just need to set sync-labels to an empty string:

sync-labels: ""

Obviously, this should be fixed but no need to drop the action when there's workaround available :P

[tupui]

@tupui the workaround for this is fairly easy, you just need to set sync-labels to an empty string:

sync-labels: ""

Obviously, this should be fixed but no need to drop the action when there's workaround available :P

Oh thanks! We sadly dropped it a few months ago already because there was no action there... I just got notified about this with the recent message and thought I would comment again to say we had to take action–as I feared we might need to.

A "hack" might not make our maintainer team confident I am afraid. I will keep an eye on this.

[jjerphan]

Hi @pje,

This is a small gentle up: is there anything to be done for this fix to be merged?

Major libraries' CI (like SciPy's and scikit-learn's) would largely benefit from this fix. 🙏

[MaksimZhukov]

We can use the getBooleanInput core function to parse a boolean input:

Suggested change

	const syncLabels =
	core.getInput("sync-labels", { required: false }) === "true";
	const syncLabels = core.getBooleanInput('sync-labels');

[adam-azarchs]

The problem with using getBooleanInput is that it will reject an empty string argument, which will break everyone currently using the workaround of setting sync-labels: ''. That's why in #480 (which yes, is a duplicate) I did something more complex to get around that. While that's still technically a breaking change, it's unlikely to break anyone who wasn't already broken.

[MaksimZhukov]

Hello @epuertat!
Thank you for the contribution and sorry for the late review!
I left a comment, please take a look. Also, please rebuild dist folder and resolve conflicts.

[jjerphan]

@epuertat: Thank you for this contribution, I am looking forward to it being integrated. Are you still working on it? If not, can I supersede it, @MaksimZhukov? 🙂

[epuertat]

@jjerphan @MaksimZhukov @adam-azarchs: please feel free to take over and move forward with this.

[MaksimZhukov]

Hello @epuertat ! Thank you for the response!
I am closing the PR in favor of this one. @jjerphan please find the details in that PR.