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 Autolabeler Config to Version 5.0.0 #3695

Merged
merged 6 commits into from
Dec 26, 2023

Conversation

Sdfendor
Copy link
Contributor

@Sdfendor Sdfendor commented Dec 7, 2023

Description (*)

Github Autolabeler updated to version 5.0.0 three days ago which breaks existing configuration files.
image

This PR adjusts the config so that it works again. No other changes regarding e.g. orphaned entries or new capabilities of new version of Autolabeler are part of this PR.

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

Flyingmana
Flyingmana previously approved these changes Dec 7, 2023
Copy link
Contributor

@tobihille tobihille left a comment

Choose a reason for hiding this comment

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

Looks good.
Since you're contributing at least on a semi-regular basis to open mage: do you want to take the opportunity to add you to the contributors?
(Just a thought, I don't know if there are rules for this and if you really want to 😅)

@Sdfendor
Copy link
Contributor Author

Sdfendor commented Dec 7, 2023

@tobihille I always assumed I've added myself already. Doesn't seem like it though.

@fballiano
Copy link
Contributor

@Sdfendor should it be this way?

Screenshot 2023-12-07 alle 13 29 12

@fballiano fballiano self-requested a review December 7, 2023 13:29
@Sdfendor
Copy link
Contributor Author

Sdfendor commented Dec 7, 2023

@fballiano This is the old state, the current run - as it is my understanding - uses the main autolabeler config not the one from PR (so in this PR the error is still visible). So only after this is merged new/existing PRs will start using the new config in autolabeler runs.

fballiano
fballiano previously approved these changes Dec 7, 2023
@fballiano
Copy link
Contributor

I've tested locally with act (which took me a good half hour to setup) and I still get

[Labeler/Add labels] ❗ ::error::Error: found unexpected type for label 'Environment' (should be array of config options)

@fballiano
Copy link
Contributor

[Labeler/Add labels] [DEBUG] Working directory '/Users/fab/Projects/magento-lts'
[Labeler/Add labels]   💬  ::debug::looking for pr #3695
[Labeler/Add labels]   💬  ::debug::fetching changed files for pr #3695
[Labeler/Add labels]   💬  ::debug::found changed files:
[Labeler/Add labels]   💬  ::debug::  .github/labeler.yml
[Labeler/Add labels]   💬  ::debug::  README.md
| The configuration file (path: .github/labeler.yml) was not found locally, fetching via the api
[Labeler/Add labels]   ❗  ::error::Error: found unexpected type for label 'Environment' (should be array of config options)
[Labeler/Add labels]   ❗  ::error::found unexpected type for label 'Environment' (should be array of config options)

I don't understand why it says that it can't find the labeler.yml when it's there...

anyway, i'm not opposed to merging this one and see what happens

@Sdfendor
Copy link
Contributor Author

Sdfendor commented Dec 8, 2023

I'm also not sure why this is the case. We encountered the same behavior on our current shop repo here on Github though. After merge it started to label once again. We made the same kind of adjustments (just less of them) that are present in this PR to our labeler.yml.

@sreichel
Copy link
Contributor

actions/labeler#712

@fballiano
Copy link
Contributor

since the current state of the main branch is broken anyway, and we already use "pull_request" as trigger (as people are talking about in actions/labeler#712) I think we should merge this one and see what happens, I can't be worse than the actual state.

@fballiano
Copy link
Contributor

I think we should merge this and see if it works on the other PRs

@fballiano fballiano merged commit 6254418 into OpenMage:main Dec 26, 2023
3 checks passed
@fballiano
Copy link
Contributor

it's merged and I run it on #3707 but I think it still doesn't work

Screenshot 2023-12-26 alle 00 34 06

what should we do?

@Sdfendor
Copy link
Contributor Author

Sdfendor commented Dec 26, 2023

In which way didn't it work? I can't see an error in the run in #3707 which was the main goal of this PR. The "fetching via the api" info line has been there before and is still there.
Screenshot_20231226-090011

@fballiano
Copy link
Contributor

ah! sorry but when I ran it it didn't seem to have added the javascript label, which it's actually added.. maybe I didn't see it at that time for some reason 😅 it's solved then, thank you!

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