#10201: Developer's Guide: add Stein's blog post on reviewing
--------------------------------------------------+-------------------------
Reporter: jsrn | Owner: mvngu
Type: enhancement | Status: needs_review
Priority: minor | Milestone: sage-4.6.1
Component: documentation | Keywords:
Author: Johan Sebastian Rosenkilde Nielsen | Upstream: N/A
Reviewer: Minh Van Nguyen | Merged:
Work_issues: |
--------------------------------------------------+-------------------------
Changes (by newvalueoldvalue):
* status: needs_work => needs_review
* reviewer: => Minh Van Nguyen
* author: jsrn => Johan Sebastian Rosenkilde Nielsen
Comment:
Replying to [comment:4 jsrn]:
> I don't immediately agree. I think that would muddle up the instructions
on reviewing too much; we already have two descriptions which are referred
to as more or less mandatory reading (the one in the Walkthrough and the
one in the Trac manual). I remember how much work it felt like, having to
read many pages of guide before being able to review a single patch.
The walk-through is meant to be what its name suggests: a bird's eye view
or overview of the whole Sage development process. The walk-through is
designed to provide beginners with an introductory appreciation for how to
familiarize themselves with the development process. This means that the
walk-through can only present certain information at an introductory level
and points out other sources containing detailed information. I have
uploaded a patch (on top of yours) that hopefully clarifies this point,
i.e. for detailed information on the review process, see the section
"Reviewing patches" in the Developer's Guide. Someone other than me needs
to look over that patch. I'm OK with your patch. If my patch gets a
positive review, then the whole ticket gets a positive review.
[[BR]][[BR]]
> Having a third covering almost the same is way too much. That's why I
added William's blog as a link, which gives it an air of "this is how
someone else does it as well, which you might read now or once you've
tried reviewing a few times.".
My intention was not clear. I wanted to merge William's materials into the
section "Reviewing patches", not create a new section in the Developer's
Guide that contains William's materials.
[[BR]][[BR]]
> I think William's blog entry contained three important additions to what
is already in the documentation:
> - A better explanation of "why should you review"
> - A (brief) guide on how William does it, what he considers important
and so on, giving a second aspect of Sage and reviewing.
> - Ideas on how to automate and improve the process for reviewing many
tickets.
>
> So I think the best alternative to linking to the blog entry is to clean
up the guide to have one - more comprehensive - description of the
reviewing process, compiled from the three we have now. This should
contain all the goodies of all three, eatly written into one and
structured into how to do it the first time, and how to later streamline
the process when reviewing many patches. This would probably require a
major restructuring of at least the Walkthrough and the Trac Manual,
though.
I agree with your point about putting William's materials into the
Developer's Guide; this should be a separate ticket. However, I disagree
with your argument that there are currently two separate guidelines in the
Developer's Guide for reviewing patches. At best, the materials in the
walk-through can only serve as an entry point into other parts of the
Developer's Guide where more detailed information is available. Hence, I
think the walk-through chapter is required reading for any new
contributors. If in the future they want further, detailed information on
a particular issue, they should consult the relevant section(s) of the
Developer's Guide. Please note that the above comments are made from
someone (me) who is more or less (very) familiar with the Developer's
Guide. This means that a lot of the time many issues are very clear to me
(in my mind). However, it is always a good policy to have new contributors
comment on the shortcomings of the Sage standard documentation. I hope my
reviewer patch (see [attachment:trac-10201_reviewer.patch]) has clarified
that the section "Reviewing patches" in the Developer's Guide is where
anyone should consult for detailed information on the review process.
--
Ticket URL: <http://trac.sagemath.org/sage_trac/ticket/10201#comment:5>
Sage <http://www.sagemath.org>
Sage: Creating a Viable Open Source Alternative to Magma, Maple, Mathematica,
and MATLAB
--
You received this message because you are subscribed to the Google Groups
"sage-trac" 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/sage-trac?hl=en.
