This is an archive of the discontinued Mercurial Phabricator instance.

similar: add condition to avoid Zerodivisonerror in function _score() (issue6099)
Needs RevisionPublic

Authored by akshjain.jain74 on Mar 12 2019, 2:09 PM.

Details

Reviewers
durin42
baymax
Group Reviewers
hg-reviewers

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

pulkit added a subscriber: pulkit.Mar 13 2019, 4:54 AM
pulkit added inline comments.
hgext/histedit.py
1127 ↗(On Diff #14477)

Is this change required for this patch?

Oh sorry ,my bad I think that is by mistake I will correct it now
Ithink while committing I added that file by mistake

av6 added a subscriber: av6.Mar 13 2019, 5:20 AM

I find it troubling that we now have contributors that don't follow #1 in https://www.mercurial-scm.org/wiki/ContributingChanges#Submission_checklist. Potentially #4 too (as Pulkit commented).

Seriously, the title is terrible at describing what this patch fixes and where. The commit message could briefly mention why/when this condition is even required.

mercurial/similar.py
66–67

This is some... interesting syntax. Is this valid Python?

av6 added inline comments.Mar 13 2019, 5:23 AM
mercurial/similar.py
66–67

This returns None in some cases, and code that uses _score() and score() tries to compare it to an integer. In Python3 None > 1 raises TypeError.

Sorry for the various mistake
I will correct everything in an hour thanks

akshjain.jain74 marked an inline comment as done.Mar 13 2019, 1:29 PM
akshjain.jain74 retitled this revision from Experimental features: Add condition to for float number (issue6099) to ZeroDivisionError: Add condition to avoid Zerodivisonerror due to float number (issue6099).Mar 13 2019, 1:34 PM
akshjain.jain74 updated this revision to Diff 14482.
av6 added a comment.Mar 13 2019, 2:02 PM

Okay, let's go over #1 in https://www.mercurial-scm.org/wiki/ContributingChanges#Submission_checklist once more. If you want to know what a good "topic" is, look at what other people do. How patches that get accepted generally look. How bug-fixing commits are worded.

Secondly, about that capitalization. I'm sorry if this is some sort of silly autocorrect that's doing it for you, I really hope you're not writing commit messages on a phone.

mercurial/similar.py
66–67

None > 1 is still an issue, see my comment above.

sorry for that , but can you tell me in what condition this function will return none value?

actually i am not getting what can be the proper topic for this :(

akshjain.jain74 retitled this revision from ZeroDivisionError: Add condition to avoid Zerodivisonerror due to float number (issue6099) to similar: add condition to avoid Zerodivisonerror in function _score() (issue6099).Mar 13 2019, 4:17 PM
akshjain.jain74 updated this revision to Diff 14487.
akshjain.jain74 marked an inline comment as done.Mar 13 2019, 4:17 PM

Can you try to add a test for this?

mercurial/similar.py
66–67

we can do if lengths: here.

68

no need for this else. you can return 0 without else.

In D6123#89410, @pulkit wrote:

Can you try to add a test for this?

yes i can definitely try to add test for this

thanks for the review

akshjain.jain74 marked 2 inline comments as done.Mar 15 2019, 10:19 AM
akshjain.jain74 updated this revision to Diff 14508.
mercurial/similar.py
68

yes actually 😅 i did'nt noticed that

Gentle ping on this. This has been around for a long time, what's the status of it?

baymax requested changes to this revision.Apr 28 2020, 4:18 AM

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.

:baymax:need-review-idle:

This revision now requires changes to proceed.Apr 28 2020, 4:18 AM