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

adding PR option to create-report command #329

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

dana-yaish
Copy link
Contributor

we need the pr number for the PR API request to know if the command runs in a fork PR

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Merging #329 (c07796d) into main (67efc57) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #329   +/-   ##
=======================================
  Coverage   95.46%   95.46%           
=======================================
  Files          80       80           
  Lines        2713     2715    +2     
=======================================
+ Hits         2590     2592    +2     
  Misses        123      123           
Flag Coverage Δ
python3.10 95.71% <100.00%> (+<0.01%) ⬆️
python3.11 95.71% <100.00%> (+<0.01%) ⬆️
python3.8 95.71% <100.00%> (+<0.01%) ⬆️
python3.9 95.71% <100.00%> (+<0.01%) ⬆️
smart-labels 95.46% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
codecov_cli/commands/report.py 73.68% <100.00%> (+1.46%) ⬆️
codecov_cli/services/report/__init__.py 100.00% <100.00%> (ø)

matt-codecov
matt-codecov previously approved these changes Nov 9, 2023
Copy link
Contributor

@matt-codecov matt-codecov left a comment

Choose a reason for hiding this comment

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

very tidy, looks good

Comment on lines +52 to +56
headers = (
{}
if not token and is_fork_pr(pull_request_number, decoded_slug, service)
else get_token_header_or_fail(token)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

super minor nit: when you merge, could you mention this extra little change in the commit message? just in case someone is reading through commit history searching for a bug or trying to learn how tokenless was implemented. something like adding PR option and tokenless logic to create-report command

Base automatically changed from dana/identify-fork-prs to main November 10, 2023 11:13
@dana-yaish dana-yaish dismissed matt-codecov’s stale review November 10, 2023 11:13

The base branch was changed.

@dana-yaish dana-yaish merged commit 65210ba into main Nov 13, 2023
14 checks passed
@dana-yaish dana-yaish deleted the dana/pr-option-in-create-report branch November 13, 2023 18:34
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.

make CLI aware if the PR is made from a fork repo
3 participants