➜
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2015-11-01 09:57 by Rohit Mediratta, last changed 2022-04-11 14:58 by admin.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| mywork.patch | Rohit Mediratta, 2015-11-01 09:57 | Patch to increase coverage for 3 additional lines | ||
| mywork_update.patch | Rohit Mediratta, 2015-11-21 08:59 | Updated patch showing the correct diff | review | |
| Messages (7) | |||
|---|---|---|---|
| msg253836 - (view) | Author: Rohit Mediratta (Rohit Mediratta) * | Date: 2015-11-01 09:57 | |
Opened to submit a patch. - make patchcheck succeeded - full testsuite succeeded - Old coverage Lib/calendar.py 375 54 86% 511, 519, 541, 608-699, 703 - New coverage Lib/calendar.py 375 51 86% 608-699, 703 |
|||
| msg254421 - (view) | Author: Martin Panter (martin.panter) *  |
Date: 2015-11-09 23:28 | |
Rohit, it looks like your patch is reversed. The lines with a + sign already exist; are the - lines your proposed additions? Equivalently, revision f7db966c9fee already exists; is fb9d4ccbadf0 your new proposed revision? Assuming the patch is reversed, I suggest keeping the locale=None case, perhaps as a separate test case or loop iteration. Otherwise you are throwing out one test case to add another. |
|||
| msg255047 - (view) | Author: Rohit Mediratta (Rohit Mediratta) * | Date: 2015-11-21 08:59 | |
Thanks for the comments. I did indeed have the patch reversed. I've resolved it here. Martin: I had the locale=None case in the patch. |
|||
| msg255416 - (view) | Author: Stéphane Wirtel (matrixise) * |
Date: 2015-11-26 13:14 | |
no problem about the second patch of Rohit. pass the test with default and I have tested the code in the REPL. |
|||
| msg255417 - (view) | Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-11-26 13:57 | |
The problem with Rohit's patch is that it throws out existing test case. |
|||
| msg255423 - (view) | Author: Stéphane Wirtel (matrixise) * |
Date: 2015-11-26 16:22 | |
Sure, But the patch is correct. Now, you are right, we have to ask him a new patch where the function is really tested. |
|||
| msg415815 - (view) | Author: Irit Katriel (iritkatriel) * |
Date: 2022-03-22 21:27 | |
The patch needs to be reviewed. If the tests are still relevant and increase coverage, it needs to be converted to a GitHub PR. Otherwise this issue can be closed. See also issue13330. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:58:23 | admin | set | github: 69714 |
| 2022-03-22 21:27:14 | iritkatriel | set | nosy:
+ iritkatriel messages: + msg415815 |
| 2015-11-26 16:22:19 | matrixise | set | messages: + msg255423 |
| 2015-11-26 13:57:55 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages: + msg255417 |
| 2015-11-26 13:14:17 | matrixise | set | nosy:
+ matrixise messages: + msg255416 |
| 2015-11-21 08:59:10 | Rohit Mediratta | set | files:
+ mywork_update.patch messages: + msg255047 |
| 2015-11-09 23:28:17 | martin.panter | set | nosy:
+ martin.panter messages: + msg254421 stage: patch review |
| 2015-11-06 21:49:37 | terry.reedy | set | nosy:
+ rhettinger |
| 2015-11-01 09:57:43 | Rohit Mediratta | create | |