You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2008/10/29 11:48:42 UTC

Questions in/on the code [was: svn commit: r33900 - trunk/subversion/libsvn_wc]

On Tue, 2008-10-28 at 11:06 -0700, Greg Stein wrote:
> Why is it horrible? How are you tracking that you need to research
> these questions? Or maybe somebody else knows the answers.

It's horrible to ask questions about the code by inserting them into the
code

  1. because it's a poor channel for discussion, it implies I have no
more effective way to talk to colleagues, and useful changes to the code
base would be interleaved with useless commits of incremental
discussion;

  2. because historically such questions are rarely answered, usually
remaining in the code for years;

  3. because it presumes the questioner is right: on the occasions where
there is an answer that explains what I needed to know, and the code was
right, the result is just code churn. Only on the occasions where the
questioner had actually found a problem is the code churn then
justified.

How am I tracking that I need to research these questions? I'm not in
any formal way. Every time I read that code I recognise that I don't
understand those bits, but the questions I inserted are just a few of
many questions.

I suppose I should just ask them on the mailing list or IRC as soon as I
realise I haven't been able to find an answer for myself.

My intention at the time was that I and others should thoroughly review
all the code changes on the branch (and so see and resolve those
questions) before merging the branch back to trunk. But I got to feel
the pressure that the branch had languished far too long and the only
way to progress it was to merge it to trunk first.


- Julian


> On Mon, Oct 27, 2008 at 9:19 AM,  <ju...@tigris.org> wrote:
> > Remove some question-comments that I inserted in the tree-conflicts branch
> > and then merged to trunk without resolving. (I still don't know the answers
> > but inserting question-comments is horrible.)
> >
> > * subversion/libsvn_wc/update_editor.c
> >  (do_entry_deletion, add_directory, add_file): Delete question-comments.
[...]
> > @@ -1359,13 +1359,13 @@ do_entry_deletion(struct edit_baton *eb,
> >
> >       tmp_entry.revision = *(eb->target_revision);
> >       tmp_entry.kind =
> > -        (entry->kind == svn_node_file) ? svn_node_file : svn_node_dir;  /* ### redundant? */
> > +        (entry->kind == svn_node_file) ? svn_node_file : svn_node_dir;
> >       tmp_entry.deleted = TRUE;
> >
> >       SVN_ERR(svn_wc__loggy_entry_modify(&log_item, adm_access,
> >                                          full_path, &tmp_entry,
> >                                          SVN_WC__ENTRY_MODIFY_REVISION
> > -                                         | SVN_WC__ENTRY_MODIFY_KIND  /* ### redundant change? */
> > +                                         | SVN_WC__ENTRY_MODIFY_KIND
> >                                          | SVN_WC__ENTRY_MODIFY_DELETED,
> >                                          pool));
> >
> > @@ -1550,7 +1550,6 @@ add_directory(const char *path,
> >
> >           /* Anything other than a dir scheduled for addition without
> >              history is an error. */
> > -          /* ### what's this "add_existed" clause for? */
> >           if (entry
> >               && entry->schedule == svn_wc_schedule_add
> >               && ! entry->copied)
> > @@ -2541,8 +2540,6 @@ add_file(const char *path,
> >   /* When adding, there should be nothing with this name unless unversioned
> >      obstructions are permitted or the obstruction is scheduled for addition
> >      (or replacement) without history. */
> > -  /* ### " or the obstruction is scheduled for addition
> > -     without history." ??? */
> >   if (kind != svn_node_none)
> >     {
> >       if (eb->allow_unver_obstructions
> >



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Questions in/on the code [was: svn commit: r33900 - trunk/subversion/libsvn_wc]

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Wed, Oct 29, 2008 at 9:34 AM, Greg Stein <gs...@gmail.com> wrote:
> If I'm reading code, then I *want* those questions in there. It tells
> me to be wary. To find out more. That what I'm looking at my be
> suspect. It is INFORMATIVE.

At the risk of sounding like a broken AOL bot, ditto again.  =)

It's a "here be dragons" caution light - it probably also indicates
that whomever wrote the code initially didn't do a very good job of
documenting/commenting the code in question.  But, at least leaving a
breadcrumb comment would do just a smidge to resolving this.  --
justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Questions in/on the code [was: svn commit: r33900 - trunk/subversion/libsvn_wc]

Posted by Greg Stein <gs...@gmail.com>.
On Wed, Oct 29, 2008 at 5:10 AM, Julian Foad <ju...@btopenworld.com> wrote:
> On Wed, 2008-10-29 at 05:04 -0700, Greg Stein wrote:
>> On Wed, Oct 29, 2008 at 4:48 AM, Julian Foad <ju...@btopenworld.com> wrote:
>> > On Tue, 2008-10-28 at 11:06 -0700, Greg Stein wrote:
>> >> Why is it horrible? How are you tracking that you need to research
>> >> these questions? Or maybe somebody else knows the answers.
>> >
>> > It's horrible to ask questions about the code by inserting them into the
>> > code
>> >
>> >  1. because it's a poor channel for discussion, it implies I have no
>> > more effective way to talk to colleagues, and useful changes to the code
>>
>> In short: you don't. And that is the problem that questions like that address.
>>
>> It's 5am here, so I'm not going to go on at length. But basically,
>> your alternatives in the rest of this email are all very less
>> effective than leaving those questions in there for others to see, be
>> wary of, to answer, or to remove because *they* know the answer is "no
>> problem".
>>
>> Removing the questions now implies that we have unknown potential
>> problems. Areas that you worked on but are NOT SURE that you got
>> right, but there is no documentation for that fact. Other than in your
>> head. And that is an awful state for us to be in.
>>
>> You CANNOT depend upon code reviews. Period. That isn't the answer to
>> resolving your questions.
>
> OK, thanks for the advice. I'm asking on-list now.

Umm... that is NOT what I was suggesting. Keep the questions in the
code. Go ahead and followup on list, but the mailing list might not
ever generate an answer, or the thread could peter out, or you might
not ever get a chance to flow the results back into the code. In all
those cases, it is best to leave the question there in the code.

If I'm reading code, then I *want* those questions in there. It tells
me to be wary. To find out more. That what I'm looking at my be
suspect. It is INFORMATIVE.

Absence of that question? Potentially dangerous. Certainly not informative.

If you have 10 questions, then by all means... put them in the code!

But please... do not simply omit them, and consider "asking on-list"
is better :-(

Cheers,
-g

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Questions in/on the code [was: svn commit: r33900 - trunk/subversion/libsvn_wc]

Posted by Julian Foad <ju...@btopenworld.com>.
On Wed, 2008-10-29 at 05:04 -0700, Greg Stein wrote:
> On Wed, Oct 29, 2008 at 4:48 AM, Julian Foad <ju...@btopenworld.com> wrote:
> > On Tue, 2008-10-28 at 11:06 -0700, Greg Stein wrote:
> >> Why is it horrible? How are you tracking that you need to research
> >> these questions? Or maybe somebody else knows the answers.
> >
> > It's horrible to ask questions about the code by inserting them into the
> > code
> >
> >  1. because it's a poor channel for discussion, it implies I have no
> > more effective way to talk to colleagues, and useful changes to the code
> 
> In short: you don't. And that is the problem that questions like that address.
> 
> It's 5am here, so I'm not going to go on at length. But basically,
> your alternatives in the rest of this email are all very less
> effective than leaving those questions in there for others to see, be
> wary of, to answer, or to remove because *they* know the answer is "no
> problem".
> 
> Removing the questions now implies that we have unknown potential
> problems. Areas that you worked on but are NOT SURE that you got
> right, but there is no documentation for that fact. Other than in your
> head. And that is an awful state for us to be in.
> 
> You CANNOT depend upon code reviews. Period. That isn't the answer to
> resolving your questions.

OK, thanks for the advice. I'm asking on-list now.

- Julian



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Questions in/on the code [was: svn commit: r33900 - trunk/subversion/libsvn_wc]

Posted by Greg Stein <gs...@gmail.com>.
On Wed, Oct 29, 2008 at 4:48 AM, Julian Foad <ju...@btopenworld.com> wrote:
> On Tue, 2008-10-28 at 11:06 -0700, Greg Stein wrote:
>> Why is it horrible? How are you tracking that you need to research
>> these questions? Or maybe somebody else knows the answers.
>
> It's horrible to ask questions about the code by inserting them into the
> code
>
>  1. because it's a poor channel for discussion, it implies I have no
> more effective way to talk to colleagues, and useful changes to the code

In short: you don't. And that is the problem that questions like that address.

It's 5am here, so I'm not going to go on at length. But basically,
your alternatives in the rest of this email are all very less
effective than leaving those questions in there for others to see, be
wary of, to answer, or to remove because *they* know the answer is "no
problem".

Removing the questions now implies that we have unknown potential
problems. Areas that you worked on but are NOT SURE that you got
right, but there is no documentation for that fact. Other than in your
head. And that is an awful state for us to be in.

You CANNOT depend upon code reviews. Period. That isn't the answer to
resolving your questions.

Cheers,
-g

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org