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.

Sign up for GitHub

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

Jump to bottom

Fix docstring for divides to clarify that if the first return value is false then the second is undefined (and not necessarily zero as was claimed before) #1972

Merged
merged 1 commit into from
Jan 25, 2025
Merged

Fix docstring for divides to clarify that if the first return value is false then the second is undefined (and not necessarily zero as was claimed before) #1972

merged 1 commit into from
Jan 25, 2025

Conversation

thofma
Copy link
Member

@thofma thofma commented Jan 24, 2025 •
edited
Loading

As observed in oscar-system/Oscar.jl#4497 (comment):

julia> divides(5, 2)
(false, 2)

CC: @joschmitt

@codecov Codecov
Copy link

codecov bot commented Jan 24, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.20%. Comparing base (767dce5) to head (49c9d3a).
Report is 3 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master    #1972   +/-   ##
=======================================
  Coverage   88.20%   88.20%           
=======================================
  Files         119      119           
  Lines       30437    30437           
=======================================
  Hits        26848    26848           
  Misses       3589     3589

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

joschmitt
joschmitt approved these changes Jan 24, 2025
View reviewed changes
Copy link
Collaborator

@joschmitt joschmitt left a comment

Choose a reason for hiding this comment

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

Thank you!

@thofma
Copy link
Member Author

thofma commented Jan 25, 2025

Are you ok with this @fingolfin? Just pinging you, since you were working on these functions in the recent past.

@fingolfin fingolfin merged commit 7921842 into master Jan 25, 2025
29 checks passed
@fingolfin fingolfin deleted the th/fixdocs branch January 25, 2025 14:34
@fingolfin fingolfin changed the title fix: docstring for divides promises too much Fix docstring for divides to clarify that if the first return value is false then the second is undefined (and not necessarily zero as was claimed before) Jan 25, 2025
@fingolfin fingolfin added release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes doc This change adds or pertains to documentation labels Jan 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fingolfin fingolfin fingolfin approved these changes

@joschmitt joschmitt joschmitt approved these changes

@lgoettgens lgoettgens lgoettgens approved these changes

Assignees
No one assigned
Labels
doc This change adds or pertains to documentation release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@thofma @fingolfin @joschmitt @lgoettgens