[go: up one dir, main page]
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

How to only add label, not remove when PR is opened? #112

Closed
pllim opened this issue Nov 11, 2020 · 7 comments · Fixed by #480
Closed

How to only add label, not remove when PR is opened? #112

pllim opened this issue Nov 11, 2020 · 7 comments · Fixed by #480
Labels
bug Something isn't working

Comments

@pllim
Copy link
Contributor
pllim commented Nov 11, 2020

I have a labeler action set up for when PR is opened, using all your default settings. The labeler YAML is set up as such (relevant parts listed below):

Docs:
  - docs/*

coordinates:
  - '**/coordinates/**/*'

When astropy/astropy#11023 was opened, the only file changed was docs/coordinates/apply_space_motion.rst. By the labeler logic, it should apply Docs and coordinates.

However, the PR author already manually applied Docs label at creation time. As a result, the labeler ended up removing the Docs label. Is this a bug? How do I tell it to simply no-op when a label that should be applied is already there?

Screenshot 2020-11-10 234620

Hope you can help and thanks for such a useful action!
@pllim pllim mentioned this issue Nov 11, 2020
Merged
@pllim
Copy link
Contributor Author
pllim commented Nov 11, 2020 •
edited
Loading

Strangely enough, accidental removal did not happen for astropy/astropy#11028 when the file changed was only docs/conf.py and Docs label was also applied manually when PR was opened.

Screenshot 2020-11-11 101002

Screenshot 2020-11-11 101107

(The Action ran successfully but it was a no-op, as expected.)

@epuertat epuertat mentioned this issue Nov 18, 2020
Merged
3 tasks
@makecir
Copy link
makecir commented Feb 2, 2021

There hasn't been much progress in the conversation, but I also want to solve this problem.
By removing the manually attached tags, the benefits gained by labelar are diminished.

@pllim pllim mentioned this issue Mar 15, 2021
Merged
@jcs090218 jcs090218 mentioned this issue Mar 16, 2021
Open
@ericdallo
Copy link

Any news here?

@pllim pllim mentioned this issue Apr 22, 2021
Merged
@pllim pllim mentioned this issue Aug 2, 2021
Merged
9 tasks
@adam-azarchs
Copy link
Contributor

It looks like the problem is

labeler/src/labeler.ts

Line 18 in 5906282

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

which is not correct because the default value is "false", and !!"false" == true.

@adam-azarchs
Copy link
Contributor

Confirmed, you can work around this bug by setting

sync-labels: ''

in your workflow.

@pllim pllim mentioned this issue Dec 24, 2021
Merged
9 tasks
This was referenced Feb 9, 2022
Closed
Merged
@dhaus67 dhaus67 mentioned this issue Mar 4, 2022
Merged
@cole-h cole-h mentioned this issue Mar 16, 2022
Closed
Gustry added a commit to 3liz/lizmap-web-client that referenced this issue Apr 11, 2022
@Gustry
GH Action - Fix the labeler about existing labels
4ce5b5c 
actions/labeler#112 (comment)
Gustry added a commit to Gustry/QGIS that referenced this issue Apr 11, 2022
@Gustry
Fix the GH Action labeler to not remove labels
aa8414a 
actions/labeler#112 (comment)
@Gustry Gustry mentioned this issue Apr 11, 2022
Merged
Gustry added a commit to Gustry/QGIS that referenced this issue Apr 11, 2022
@Gustry
Fix the GH Action labeler to not remove labels
00bec4f 
actions/labeler#112 (comment)
Gustry added a commit to Gustry/QGIS that referenced this issue Apr 11, 2022
@Gustry
Fix the GH Action labeler to not remove labels
56a55ab 
actions/labeler#112 (comment)
nyalldawson pushed a commit to qgis/QGIS that referenced this issue Apr 11, 2022
@Gustry @nyalldawson
Fix the GH Action labeler to not remove labels
1e560b3 
actions/labeler#112 (comment)
@liverpool1026
Copy link

Confirmed, you can work around this bug by setting

sync-labels: ''

in your workflow.

Thanks for this, can also confirm this works in v4. Any plans to fix this properly in the next version?

@fanny-jiang fanny-jiang mentioned this issue Sep 26, 2022
Merged
4 tasks
This was referenced Oct 31, 2022
Closed
Closed
@MaksimZhukov MaksimZhukov added the bug Something isn't working label Dec 12, 2022
This was referenced Dec 12, 2022
Closed
Closed
@Illviljan Illviljan mentioned this issue Jan 10, 2023
Merged
@adam-azarchs adam-azarchs mentioned this issue Jan 11, 2023
Merged
2 tasks
adam-azarchs added a commit to adam-azarchs/labeler that referenced this issue Jan 18, 2023
@adam-azarchs
fix: correct reading of sync-labels input.
613b00b 
Contrary to the assumptions made in the unit tests, core.getInput
always returns a string, and !!'false' is true.

Also updates the unit test to reduce changes of regressions by ensuring
that the mocked getInput returns a string, as it would in production.

Fixes actions#112

Make sure test catches regressions.
rickstaa added a commit to rickstaa/github-emoji-picker that referenced this issue Feb 7, 2023
@rickstaa
ci: prevent label remove
6fc71c9 
This prevents the removal of labels that occurs due to a bug in 'actions/labeler' (see actions/labeler#112).
@rickstaa rickstaa mentioned this issue Feb 7, 2023
Merged
rickstaa added a commit to rickstaa/github-emoji-picker that referenced this issue Feb 7, 2023
@rickstaa
ci: prevent label remove (#69)
25b2e51 
This prevents the removal of labels that occurs due to a bug in 'actions/labeler' (see actions/labeler#112).
@foooomio foooomio mentioned this issue Feb 24, 2023
Merged
MaksimZhukov pushed a commit that referenced this issue Mar 23, 2023
@adam-azarchs
fix: correct reading of sync-labels input. (#480)
751921c 
* fix: correct reading of sync-labels input.

Contrary to the assumptions made in the unit tests, core.getInput
always returns a string, and !!'false' is true.

Also updates the unit test to reduce changes of regressions by ensuring
that the mocked getInput returns a string, as it would in production.

Fixes #112

Make sure test catches regressions.

* Rebuild dist/index.js.
@Illviljan Illviljan mentioned this issue Mar 23, 2023
Merged
@luisorbaiceta luisorbaiceta mentioned this issue Apr 5, 2023
Closed
@aliok aliok mentioned this issue Jun 2, 2023
Merged
CaseyHillers pushed a commit to flutter/flutter that referenced this issue Jul 14, 2023
[labeler] Mark sync-labels as empty
339ee1b 
* Based on actions/labeler#112 (comment)
@CaseyHillers CaseyHillers mentioned this issue Jul 14, 2023
Merged
8 tasks
auto-submit bot pushed a commit to flutter/flutter that referenced this issue Jul 18, 2023
[labeler] Mark sync-labels as empty (#130642)
ab14a5c 
#128440

Recommendation from actions/labeler#112 (comment)
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this issue Jul 31, 2023
@LouiseHsu
[labeler] Mark sync-labels as empty (flutter#130642)
c125db7 
flutter#128440

Recommendation from actions/labeler#112 (comment)
@Czaki Czaki mentioned this issue Oct 17, 2023
Merged
Czaki added a commit to napari/napari that referenced this issue Oct 18, 2023
@Czaki
Fix labeler configuration (#6344)
efa06ad 
# Description

In PR #6328 I have added an automatic maintenance label when something
is edited pre-commit. But I wrongly understand actions/lableler
documentation and thought that the bug was fixed, that is true, but it
is fixed in v5 branch, not backported to v4.

v5 has been from a long time (since May) in the release process.

actions/labeler#112
@agriyakhetarpal agriyakhetarpal mentioned this issue Nov 20, 2023
Closed
@drewroengoogle drewroengoogle mentioned this issue Dec 4, 2023
Merged
8 tasks
auto-submit bot pushed a commit to flutter/flutter that referenced this issue Dec 4, 2023
@drewroengoogle
Update sync-labels to use boolean (#139516)
e1a2bb3 
actions/labeler#112 has been fixed, and now requires boolean to be used. 

Related infra issue that this fixes: #139511
@auto-submit auto-submit bot mentioned this issue Dec 5, 2023
Merged
8 tasks
auto-submit bot added a commit to flutter/flutter that referenced this issue Dec 5, 2023
@auto-submit
Reverts "Update sync-labels to use boolean" (#139533)
5422af7 
Reverts #139516
Initiated by: godofredoc
This change reverts the following previous change:
Original Description:
actions/labeler#112 has been fixed, and now requires boolean to be used. 

Related infra issue that this fixes: #139511
@greschd greschd mentioned this issue Dec 6, 2023
Merged
@agriyakhetarpal agriyakhetarpal mentioned this issue Dec 21, 2023
Merged
nyalldawson added a commit to qgis/QGIS that referenced this issue Jan 2, 2024
@nyalldawson
Fix labeler workflow
3728a24 
Refs actions/labeler#112
@jenssimon jenssimon mentioned this issue Mar 13, 2024
Closed
fmigneault-crim referenced this issue in Ouranosinc/Magpie Apr 26, 2024
@fmigneault
patch github action labeler - leave manual labels (https://github.com…
154736d 
…/actions/labeler/issues/112\#issuecomment-1000491676)
@xSAVIKx
Copy link
xSAVIKx commented Jul 19, 2024

This still may happen with v5 action if the labels are added by the user at the time that's very close to the action run:
image

User added labels at 2024-07-19T15:07:34Z. Bot added his own at 2024-07-19T15:07:35Z.

And bot started running at 2024-07-19T15:07:34Z as per GH action logs. So my guess is that it read labels, then user added his labels, then bot applied the rules and has overridden some of the labels the user has previously added.

WF config:

# .github/workflows/auto-labeler.yaml
name: "Pull Request Labeler"
on:
  pull_request_target:
    types: [ opened, reopened, synchronize ]

jobs:
  labeler:
    permissions:
      contents: read
      pull-requests: write
    runs-on: ubuntu-22.04
    steps:
      - uses: actions/labeler@v5
        with:
          sync-labels: false
          dot: true
          configuration-path: .github/labeler.yaml

Related rules:

"/web":
  - any:
      - changed-files:
          - any-glob-to-any-file:
              - SOME FILES
javascript:
  - any:
      - changed-files:
          - any-glob-to-any-file:
              - src/js/**

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: correct reading of sync-labels input. adam-azarchs/labeler
7 participants
@pllim @xSAVIKx @ericdallo @adam-azarchs @liverpool1026 @makecir @MaksimZhukov