Skip to content

Conversation

hectorhdzg
Copy link
Member

@hectorhdzg hectorhdzg commented Apr 23, 2025

Which problem is this PR solving?

Automate label tagging for owner reviews in PRs

Short description of the changes

Added scripts to parse component_owners.yml, to get existing components and owners by componer
Added script to get details about a PR, looks for all updated files and determine if one of the owner of that file already approved the PR, then add label.
Added workflow to kick off the script when a PR is reviewed and approved.

Copy link

codecov bot commented Apr 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.74%. Comparing base (2317e2f) to head (36b8aa7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2798      +/-   ##
==========================================
+ Coverage   89.69%   89.74%   +0.04%     
==========================================
  Files         186      186              
  Lines        9048     9048              
  Branches     1855     1855              
==========================================
+ Hits         8116     8120       +4     
+ Misses        932      928       -4     

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hectorhdzg hectorhdzg changed the title Add Github action to add "has:owner-approval" label for approved PRs ci: Add Github action to add "has:owner-approval" label for approved PRs Apr 23, 2025
@hectorhdzg hectorhdzg marked this pull request as ready for review May 2, 2025 21:39
@hectorhdzg hectorhdzg requested a review from a team as a code owner May 2, 2025 21:39
Copy link
Contributor

@JacksonWeber JacksonWeber left a comment

Choose a reason for hiding this comment

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

LGTM

@pichlermarc
Copy link
Member

overall looks good, let's use some explicit permissions and then we can get this merged. 🙂

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

see previous comment

@hectorhdzg
Copy link
Member Author

Updated, thanks for reviewing @pichlermarc let me know if there is something else

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

ah - looks like the script is missing +x permissions.

@pichlermarc
Copy link
Member

(see this run)

Copy link
Member

@pichlermarc pichlermarc left a comment

Choose a reason for hiding this comment

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

see previous comments

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.

3 participants