Noorul Islam K M wrote on Sat, Feb 05, 2011 at 12:55:55 +0530:
> Daniel Shahaf <d.s_at_daniel.shahaf.name> writes:
> > Noorul Islam K M wrote on Thu, Feb 03, 2011 at 14:15:48 +0530:
> >> + if (info->depth == svn_depth_exclude)
> >> + SVN_ERR(svn_cmdline_printf(pool, _("Depth: exclude\n")));
> >>
> >
> > I know that there is already some other place in the code that prints
> > the Depth: line, why didn't you reuse that place?
> >
>
> It is inside the if block 'if (info->has_wc_info)'. In our case flow
> will not reach there so I had to put it here.
>
... and? Why won't it enter that block? i.e., why is has_wc_info false?
Some of the fields there (eg, checksum and copyfrom) make perfect sense
even for excluded nodes. (I don't know whether the wc can supply those
values even for excluded nodes.)
> >> + if (tree_conflict || exclude)
> >> {
> >> svn_info_t *info;
> >> svn_error_clear(err);
> >>
> >> SVN_ERR(build_info_for_unversioned(&info, pool));
> >> - info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
> >>
> >> + if (tree_conflict)
> >> + info->tree_conflict = svn_wc__cd2_to_cd(tree_conflict, pool);
> >> + else
> >> + info->depth = svn_depth_exclude;
> >
> > s/else/if (exclude)/, please.
> >
>
> There is actually an outer if condition which makes this not useful.
I know (it's about 5 lines above this line). But IMO this way is more
readable/maintainable.
> >> +
> >> SVN_ERR(svn_wc__node_get_repos_info(&(info->repos_root_URL),
> >> - NULL,
> >> + exclude ?
> >> + &(info->repos_UUID) : NULL,
> >
> > Why?
> >
>
> I thought I should not make changes to existing behaviour. I think it is
> safe to just pass &(info->repos_UUID) in both cases.
>
... and? What is the "change to existing behaviour" you're talking about? (I
guess it's printing the repository UUID for excluded nodes?)
Received on 2011-02-08 03:14:01 CET