float and int parsing and conversion for 5.0 #64
Conversation
|
If you would like a couple of tests would be very welcome to make sure we catch these out |
|
@musm don't do "Rebase and merge," github does not include the reference to the PR # in the commit message when you do that. And you should generally leave PR's open for longer on packages that you're not the sole author of. I've warned you about this before. |
|
@tkelman, forgive me for intruding. I don't contribute to this package. But, your last comment is a bit strange. If a collaborator (which implies status) wants to merge in a pull request then they shouldn't have to wait for some arbitrary length of time. I think it's important to remember that Julia exists in a non-profit context and we're all contributing our time because we think our work is valuable. No one out here is rushing to merge PRs to meet deadlines (because we don't have them). If a PR is merged in, then it must be that the work was thought to be beneficial to the project. Also, with the power of git it's easy to revert small PRs if we really have to. |
|
@musm has not had contributor access here for long at all. Code review matters, particularly people who are depending on this package. |
|
Right, but because he (gender assumption) is a contributor it means that he's trusted. If I'm not mistaken, you're also not the project maintainer. Also, I see that the merge failed to pass CI testing, but that doesn't imply there wasn't code review. The tests may need to be changed to reflect that the package is working. In that case, the changes to the useable part of the code base would be fine. I respect your ideas, I just think that this particular commit may have been a weak place to voice them. All that happened was the introduction of How long would you want someone to wait before merging in a PR as a contributor? |
|
In this commit, The nightly failures are unrelated and due to a new world error related to callbacks. The tests pass on 0.4 and 0.5. The PR is uncontroversial bug fix (thank you @beOn). |
|
Sure thing :)
…On Wed, Jan 11, 2017 at 2:44 PM Mus M ***@***.***> wrote:
In this commit, parsefloat was deprecated in 0.5 and thus these methods
no longer work on 0.5 (it wasn't being tested for). Similar with the other
changes.
The nightly failures are unrelated and due to a new world error related to
callbacks. The tests pass on 0.4 and 0.5.
The PR is uncontroversial bug fix (thank you @beOn
<https://github.com/beOn>).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#64 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC-RIqROK7DdgY_B8f_cA0nxL1NRt2Zks5rRT81gaJpZM4LgpML>
.
|
|
People who contribute to Julia and its packages are based all around the world. Given time zones, waiting at least a day for anything non-urgent to allow people to see, be aware of, and comment on a PR would be sufficient and recommended. Given the lack of test coverage hitting these lines, it would have been very useful to add a test here. |
|
Alas.
…On Wed, Jan 11, 2017 at 3:00 PM Tony Kelman ***@***.***> wrote:
People who contribute to Julia and its packages are based all around the
world. Given time zones, waiting at least a day for anything non-urgent to
allow people to see, be aware of, and comment on a PR would be sufficient
and recommended.
Given the lack of test coverage hitting these lines, it would have been
very useful to add a test here.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#64 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAC-RKX6qXEKnZqui2xioTdaII6T3ZBSks5rRULwgaJpZM4LgpML>
.
|
No description provided.