Skip to content

bpo-25735: math.factorial doc should mention integer return type#6420

Merged
csabella merged 2 commits into
python:masterfrom
akshaysharma096:bpo-25735
May 31, 2019
Merged

bpo-25735: math.factorial doc should mention integer return type#6420
csabella merged 2 commits into
python:masterfrom
akshaysharma096:bpo-25735

Conversation

@akshaysharma096

@akshaysharma096 akshaysharma096 commented Apr 8, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

@the-knights-who-say-ni

the-knights-who-say-ni commented Apr 8, 2018

Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@akshaysharma096

akshaysharma096 commented Apr 9, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@the-knights-who-say-ni CLA has been signed. Url: https://secure.na1.echosign.com/document/cp/S3AAABLblqZhB1NMLLtxWOEixqwtecFg/Python_Contributor_Agreement_Form.pdf&il=true

@chadmiller-amzn

chadmiller-amzn commented Apr 9, 2018 •
edited
Loading

Copy link
Copy Markdown

The bot isn't that smart yet. File a bug report in the bot's source's github page, asking for a manual re-run.

@chadmiller-amzn

chadmiller-amzn commented Apr 9, 2018

Copy link
Copy Markdown

Is this necessary? I don't think anyone could realistically think it's not an integer.

@akshaysharma096

akshaysharma096 commented Apr 14, 2018

Copy link
Copy Markdown
Contributor Author

@chadmiller-amzn Yes, it is, but always better to specify to save developer time.

mcscope
mcscope reviewed May 15, 2018
View reviewed changes
Comment thread Misc/NEWS.d/next/Documentation/2018-04-08-19-09-22.bpo-25735.idVQBD.rst Outdated
@@ -0,0 +1 @@
Added documentations for :func:`fabs`, to take only integer input values.

@mcscope mcscope May 15, 2018

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.

shouldn't this say "added documentation for func factorial to indicate that returns integer values" ?

@akshaysharma096 akshaysharma096 May 17, 2018

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.

Noted will do

@akshaysharma096 akshaysharma096 May 17, 2018

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.

@mcscope you can check again

jdemeyer
jdemeyer requested changes Feb 4, 2019
View reviewed changes
Comment thread Misc/NEWS.d/next/Documentation/2018-04-08-19-09-22.bpo-25735.idVQBD.rst
@@ -0,0 +1 @@
Added documentation for func factorial to indicate that returns integer values

@jdemeyer jdemeyer Feb 4, 2019

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.

Do we really need a NEWS entry for adding 3 words to the doc? I don't think so.

csabella
csabella approved these changes May 31, 2019
View reviewed changes

@csabella csabella left a comment

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.

LGTM

@csabella csabella closed this May 31, 2019
@csabella csabella reopened this May 31, 2019
@miss-islington

miss-islington commented May 31, 2019

Copy link
Copy Markdown
Contributor

@akshaysharma096: Status check is done, and it's a failure ❌ .

1 similar comment
@miss-islington

miss-islington commented May 31, 2019

Copy link
Copy Markdown
Contributor

@akshaysharma096: Status check is done, and it's a failure ❌ .

@csabella csabella added the docs Documentation in the Doc dir label May 31, 2019
@miss-islington

miss-islington commented May 31, 2019

Copy link
Copy Markdown
Contributor

@akshaysharma096: Status check is done, and it's a failure ❌ .

@csabella csabella merged commit 4612671 into python:master May 31, 2019
@csabella

csabella commented May 31, 2019

Copy link
Copy Markdown
Contributor

@akshaysharma096, thank you for the contribution. It has now been merged! 🎉

@miss-islington

miss-islington commented May 31, 2019

Copy link
Copy Markdown
Contributor

Thanks @akshaysharma096 for the PR, and @csabella for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 31, 2019
@akshaysharma096 @miss-islington
bpo-25735: math.factorial doc should mention integer return type (pyt…
909b4e9 
…honGH-6420)

(cherry picked from commit 4612671)

Co-authored-by: Akshay Sharma <akshay.sharma09695@gmail.com>
@bedevere-bot

bedevere-bot commented May 31, 2019

Copy link
Copy Markdown

GH-13702 is a backport of this pull request to the 3.7 branch.

miss-islington added a commit that referenced this pull request May 31, 2019
@miss-islington @akshaysharma096
bpo-25735: math.factorial doc should mention integer return type (GH-…
fc3b843 
…6420)

(cherry picked from commit 4612671)

Co-authored-by: Akshay Sharma <akshay.sharma09695@gmail.com>
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
@akshaysharma096 @DinoV
bpo-25735: math.factorial doc should mention integer return type (pyt…
17d580f 
…honGH-6420)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdemeyer jdemeyer jdemeyer requested changes
@csabella csabella csabella approved these changes
+1 more reviewer
@mcscope mcscope mcscope left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs Documentation in the Doc dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@akshaysharma096 @the-knights-who-say-ni @chadmiller-amzn @miss-islington @csabella @bedevere-bot @jdemeyer @mcscope @ned-deily