Skip to content

Add name filter to Repository.get_artifacts()#2459

Merged
EnricoMi merged 9 commits intoPyGithub:masterfrom
trim21:list-artifacts
Jun 1, 2023
Merged

Add name filter to Repository.get_artifacts()#2459
EnricoMi merged 9 commits intoPyGithub:masterfrom
trim21:list-artifacts

Conversation

@trim21
Copy link
Contributor

@trim21 trim21 commented Mar 19, 2023

@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (1081638) 98.68% compared to head (da83c93) 98.68%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2459   +/-   ##
=======================================
  Coverage   98.68%   98.68%           
=======================================
  Files         117      117           
  Lines       11823    11825    +2     
=======================================
+ Hits        11668    11670    +2     
  Misses        155      155           
Impacted Files Coverage Δ
github/Repository.py 97.20% <100.00%> (+<0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
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
Copy link
Collaborator

@trim21 can you add a test for this please?

@trim21
Copy link
Contributor Author

trim21 commented May 13, 2023

@trim21 can you add a test for this please?

done

@EnricoMi EnricoMi changed the title feat: add Repository.get_artifacts() name query Add Repository.get_artifacts() name query May 23, 2023
repr(artifact), 'Artifact(name="vscode-codeql-extension", id=10495898)'
)

def testGetArtifactsFromWorkflowWithName(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your test name suggests this gets artifacts for a workflow. Given the name, we would expect only a single artifact. That is the reason for my confusion. Test should be renamed:

Suggested change
def testGetArtifactsFromWorkflowWithName(self):
def testGetArtifactsFromRepoWithName(self):

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you consider to add the name filter to Repository.get_workflow_run(...).get_artifacts as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe later in another PR, not in this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after pyi are merged back to py

trim21 and others added 3 commits June 1, 2023 16:45
Co-authored-by: Enrico Minack <github@enrico.minack.dev>
Co-authored-by: Enrico Minack <github@enrico.minack.dev>
Copy link
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 changed the title Add Repository.get_artifacts() name query Add name filter to Repository.get_artifacts() Jun 1, 2023
@EnricoMi EnricoMi merged commit 9f52e94 into PyGithub:master Jun 1, 2023
@trim21 trim21 deleted the list-artifacts branch July 18, 2023 10:50
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