Am 02.04.2012 08:15, schrieb Aaron Meurer:
On Sun, Apr 1, 2012 at 11:35 PM, Joachim Durchholz<[email protected]> wrote:
You'll have to explain what "atomic" is. This can range from five-liners
tofive-thousand-liners.
That's my point exactly.
My point here is that a five-LoC commit, even if it's atomic, isn't worth
it.
And a 5000-LoC commit, no matter how well it fits the "atomic" definition,
is not reviewable and should be redesigned so that smaller atomic commits
can be made.
If it's a five-line orthoganl bugfix, this should probably be
submitted as a separate pull request.
We are not talking about an independent bugfix. If it were an
independent bugfix, there would not be any doubt about its size.
> I agree that such a small
change within the project is probably too small for a single pull
request (that's only useful if you don't plan on doing any additional
work, which won't be the case for GSoC projects).
Exactly. We're discussing recommendations for GSoC students.
On a related point: To use atomicity as a criterion, we need a definition.
My definition would be: "Cannot be broken into pieces without losing
property XYZ." However, I'm coming up blank with what XYZ is, and without
that, the criterion cannot be applied. In fact I think the definition of XYZ
is the salient point here.
I was thinking that XYZ would be something along the lines of a
passing test suite for the function. That may sound silly, but this
is exactly when the code works, which means that it can be submitted.
If the tests don't pass (assuming you've written all test cases
already), then you still need to work on the code.
That's a definition of "production-ready" - which, of course, any pull
request must satisfy to go in.
It's not a definition of "atomic" though. A 50,000-line pull request can
have a complete test suite and pass all tests, yet I'd hardly consider
such a thing atomic.
And as I said, this will encourage good coding practices like writing
tests before code, writing tests for internal functions (instead of
just for the public interface), and writing documentation with the
code.
I agree that a test suite should be part of the recommendations.
I have been assuming it's already there.
By the way, another idea we could try is to use a time based metric
instead of a size one. So, for example, we could say that once a week
(say), the student should submit what he has done (that works).
Sounds good.
A few details:
- "Week" might be misunderstood as "exactly every seven days". I'd say
"a couple of days" for that reason.
- I'd also make clear that we're not talking about calendar days but
about days of coding. If (for a slightly exaggerated example) the
student spends five days researching a tricky algorithm, he does not
need to make a pull request of the code sketches he's done in the
remaining two days.
- I'd emphasize that the student has to organize his work so that it is
possible to pull.
So I'd say:
The student should organize his work so that something is finished after
every handful of days of coding.
"Finished" code does something useful, has a test suite that covers all
code paths and all boundary cases, passes that test suite, and does not
break any of the existing tests in SymPy.
(I'm not 100% sure about the "does something useful" bit. Some
algorithms cannot be broken down in week-sized chunks that do something
useful.)
> I
think reviewing a week's worth of code is reasonable. This should be
more accurate than line- or commit-based metrics, I think, because the
time to review code should be roughly proportional to the time it took
to write it.
Agreed.
--
You received this message because you are subscribed to the Google Groups
"sympy" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/sympy?hl=en.
