Skip to content

Add actor property to WorkflowRun#2764

Merged
EnricoMi merged 5 commits intoPyGithub:mainfrom
gbhand:patch-2
Oct 27, 2024
Merged

Add actor property to WorkflowRun#2764
EnricoMi merged 5 commits intoPyGithub:mainfrom
gbhand:patch-2

Conversation

@gbhand
Copy link
Copy Markdown
Contributor

@gbhand gbhand commented Sep 21, 2023

The GitHub API returns an actor field when listing workflow runs but WorkflowRun does not include actor.

This PR adds the actor property of NamedUser type.

Not sure about field ordering conventions so just followed the API response schema.

if "updated_at" in attributes: # pragma no branch
self._updated_at = self._makeDatetimeAttribute(attributes["updated_at"])
if "actor" in attributes: # pragma no branch
self._actor = self._makeClassAttribute(github.NamedUser.NamedUser, attributes["actor"])
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Need to import this class I guess

Copy link
Copy Markdown
Contributor Author

@gbhand gbhand Sep 22, 2023

Choose a reason for hiding this comment

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

Yeah wasn't too sure what is needed here. I saw later on github.Repository.Repository also was referenced but only imported by name with TYPE_CHECKING.

Would adding import github.NamedUser be the right move?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed 841daed

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 26, 2023

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Project coverage is 96.72%. Comparing base (b986a98) to head (841daed).
Report is 79 commits behind head on main.

Files with missing lines Patch % Lines
github/WorkflowRun.py 66.66% 3 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2764      +/-   ##
==========================================
- Coverage   96.74%   96.72%   -0.02%     
==========================================
  Files         139      139              
  Lines       14184    14193       +9     
==========================================
+ Hits        13722    13728       +6     
- Misses        462      465       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gbhand gbhand requested a review from trim21 September 26, 2023 07:02
@trim21
Copy link
Copy Markdown
Contributor

trim21 commented Sep 26, 2023

lgtm

Copy link
Copy Markdown
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

Can we add this new attribute to https://github.com/PyGithub/PyGithub/blob/main/tests/WorkflowRun.py#L40? LGTM besides this.

@EnricoMi
Copy link
Copy Markdown
Collaborator

@gbhand?

@github-actions github-actions bot added the ⭐ top pull request Top pull request. label Sep 19, 2024
@gbhand
Copy link
Copy Markdown
Contributor Author

gbhand commented Oct 10, 2024

Apologies @EnricoMi, haven't checked this in quite a while. Added to the unit test in 6347cb2.

Copy link
Copy Markdown
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

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

LGTM!

@EnricoMi EnricoMi added this pull request to the merge queue Oct 27, 2024
Merged via the queue into PyGithub:main with commit 612ba68 Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⭐ top pull request Top pull request.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants