-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
bpo-39479:Add math.lcm() function: Least Common Multiple #18547
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
Conversation
This PR is created due to some problems caused in the repository. |
It is showing an error 1 test failed: |
I recommend that you investigate Git branching: instead of making all your commits on the master branch of your fork, create a dedicated branch for the work you're doing, and make the pull request from that. Apart from anything else, that will enable you to work on multiple independent issues at once, in different branches of your fork. See the developer guide for more. |
Thanks for the information. |
======================================================================
|
Above is the error showing. |
Misc/NEWS.d/next/Library/2020-02-18-12-37-16.bpo-39479.j3UcCq.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've left a few comments.
The macOS failures are unrelated, and are described in https://bugs.python.org/issue38691. Once #18553 is merged, you should be able to merge origin/master into your branch to get the CI to pass.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Thanks for making the requested changes! @mdickinson: please review the changes made to this pull request. |
Not quite. :-) Please could you address my comment about alphabetical order in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's still one unaddressed comment from the last change request.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
I have made the requested changes; please review again |
Thanks for making the requested changes! @mdickinson: please review the changes made to this pull request. |
@ananthan-123 Please can you investigate the failure on Travis CI? I think you may need to re-run (As a general rule, it's good to try to get all the CI checks passing before asking for review.) |
Hurray!The test is passed |
I have made the requested changes; please review again |
Thanks for making the requested changes! @mdickinson: please review the changes made to this pull request. |
Codecov Report
@@ Coverage Diff @@
## master #18547 +/- ##
===========================================
+ Coverage 82.11% 83.13% +1.01%
===========================================
Files 1956 1571 -385
Lines 589382 414787 -174595
Branches 44456 44458 +2
===========================================
- Hits 483979 344832 -139147
+ Misses 95751 60332 -35419
+ Partials 9652 9623 -29
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@mdickinson: Please replace |
https://bugs.python.org/issue39479