Skip to content

Conversation

@manishprivet
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

When adding a Label to the PR, we should use the Add Label API rather than the API to update the entire issue.

The current implementation can lead to unexpected behaviour if another labelling job is running in parallel, e.g., it can remove the label added by another job if timings match.

Notes to Reviewer

How to test

Link to issues addressed

@manishprivet
Copy link
Contributor Author

@caugner do you think we can merge this PR?

@caugner
Copy link

caugner commented Jan 11, 2025

@caugner do you think we can merge this PR?

LGTM, but I don't have write access on this repo.

@johnlk might be able to approve and merge.

Copy link
Contributor

@johnlk johnlk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix!

@manishprivet
Copy link
Contributor Author

Thanks @johnlk , can you please merge as well 🙇🏼

@johnlk
Copy link
Contributor

johnlk commented Jan 15, 2025

Thanks @johnlk , can you please merge as well 🙇🏼

Weird, seems there's a PR check that's blocking. Maybe we need to update the branch protection rules or run something. @JavierCane can you support here?

@Chemaclass
Copy link
Contributor

@rgomezcasas can you check this one? It seems a small but powerful fix

@JavierCane JavierCane merged commit 4ec6770 into CodelyTV:main May 25, 2025
@JavierCane
Copy link
Member

Merging and releasing a new version as a bugfix. Sorry for not replying sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants