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

Remove division by zero error #3

Merged
merged 2 commits into from
Dec 20, 2023
Merged

Remove division by zero error #3

merged 2 commits into from
Dec 20, 2023

Conversation

AnantStrange
Copy link
Contributor

In fetch_github_stats.py line 297, if user had 0 pull request, a 0 division error would be thrown.
Placed a Check to set pull_merge_request = 0 if merge_request = 0.

@x0rzavi
Copy link
Owner

x0rzavi commented Dec 20, 2023

Please update the commit message, there's a minor typo in the file name

@AnantStrange
Copy link
Contributor Author

Corrected the Typo :)

@x0rzavi
Copy link
Owner

x0rzavi commented Dec 20, 2023

Please update the requested reviews too, otherwise looks good to merge.

Copy link
Contributor Author

@AnantStrange AnantStrange left a comment

Choose a reason for hiding this comment

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

Review done and ready to be merged.

@x0rzavi
Copy link
Owner

x0rzavi commented Dec 20, 2023

You are supposed to make the changes requested in the review 😅. As for this one, its "Can you move the checking part under the block at line 282? Otherwise, the checking would be done even if data couldn't be fetched."

@AnantStrange
Copy link
Contributor Author

Alright, Made the changes - Moved the check inside the if user_stats: block.

# Set a default value or handle the case when there are no pull requests
pull_requests_merge_percentage = 0 # You can choose another appropriate value


if user_stats:

This comment was marked as resolved.

@x0rzavi
Copy link
Owner

x0rzavi commented Dec 20, 2023

Alright, LGTM. Thank you for your contribution! :)

@x0rzavi x0rzavi merged commit c3bec5d into x0rzavi:main Dec 20, 2023
1 check passed
@AnantStrange
Copy link
Contributor Author

Thank you too, my first pr 😁.

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.

2 participants