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.

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

Update labels when config changes #22496

Closed
rarkins opened this issue May 30, 2023 · 12 comments · Fixed by #25340
Closed

Update labels when config changes #22496

rarkins opened this issue May 30, 2023 · 12 comments · Fixed by #25340
Assignees
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)

Comments

@rarkins
Copy link
Collaborator

rarkins commented May 30, 2023

Describe the proposed change(s).

Renovate should be able to support changing of labels in an existing PR, as long as the user hasn't modified the labels Renovate originally added.

We don't want to be in a situation where users remove/change labels and Renovate keeps putting them back. To avoid that, we can potentially embed a hash in the PR body which records which labels were added to the PR originally, or when they were last updated. If all such labels are still in the PR, then we should be free to update/sync the labels (including adding and removing).

Describe why we need/want these change(s).

Examples:

  • user modifies their config to add labels, when previously there were none
  • user has labels for dynamic fields like updateType or confidenceLevel

It's unintuitive today why changes can't be made in these situations.

@rarkins rarkins added type:feature Feature (new functionality) priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others status:ready labels May 30, 2023
@secustor
Copy link
Collaborator

Duplicate of #9950 ?

@rarkins
Copy link
Collaborator Author

rarkins commented May 30, 2023

Seems to be related, but a different idea. Perhaps we use my idea above but could add a new option updateLabels with values never (current behavior), auto (new default), and always (enforced)?

@PhilipAbed
Copy link
Collaborator

i think you are inviting more problems and complexity by deciding if it is updated manually or by renovate,
why dont we just create a param to override labels=true and only if the user requested this param then we do it on open PRs?

@rarkins rarkins changed the title Allow updating of labels when config changes Update labels when config changes Jul 10, 2023
@rarkins
Copy link
Collaborator Author

rarkins commented Jul 10, 2023

Reasons:

  1. This is desired behavior to have by default
  2. It's necessary to also be able to remove labels (consider merge confidence labels as an example)
  3. It wouldn't be OK if Renovate always removed any user label which was added

Hence we need to be able to distinguish between these two cases:

  • A label is additional or missing because config changed, or
  • A label is additional or missing because the user removed it

@secustor
Copy link
Collaborator

In that case we should basically add a hidden string inside of PRs/Issues to save the last applied labels.
Pretty much like the debug information we have right now.

@rarkins
Copy link
Collaborator Author

rarkins commented Jul 10, 2023

A hidden string would be simpler, a hash of that string would be shorter

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Aug 14, 2023

If we use pr-comments we can only enable this feature for github, gitlab and gitea.

Update:
Only azure is left out since we don't set labels on others ie. codecommit, bitbucket, bitbucket-server

We can handle azure but there is a catch:

issue the azure api endpoint that we use to fetch PRs returns PR but has a 400 char limit on its description
workaround set the comment with labels at top of Pr body

@rarkins
Copy link
Collaborator Author

rarkins commented Aug 14, 2023

Actually if the "hidden strings" are short hashes, I think we could make them non-hidden for platforms which don't support html comments if the functional benefits outweigh the "costs" (extra lines in the PR description)

@RahulGautamSingh
Copy link
Collaborator

RahulGautamSingh commented Aug 17, 2023

Okay, also it has to be done in 2 PRs so that the hash is added to all existing PRs before we start adding/removing labels.
Else we will reapply labels which can annoy users who have removed some labels from open PRs. (or is this okay?)

@rarkins
Copy link
Collaborator Author

rarkins commented Aug 17, 2023

We should only reapply labels to existing PRs if that PR was created with a label hash, so that we don't annoy users who have removed labels

@RahulGautamSingh
Copy link
Collaborator

Is this not ready?

@rarkins
Copy link
Collaborator Author

rarkins commented Oct 9, 2023

We no longer use status:ready - if an issue has no status:* label then it's ready

@RahulGautamSingh RahulGautamSingh self-assigned this Oct 19, 2023
@rarkins rarkins added the status:in-progress Someone is working on implementation label Mar 18, 2024
@RahulGautamSingh RahulGautamSingh removed the status:in-progress Someone is working on implementation label Apr 4, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
priority-3-medium Default priority, "should be done" but isn't prioritised ahead of others type:feature Feature (new functionality)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants