[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Moved EOL functions to new libsvn_subr/eol.c

From: Stefan Sperling <stsp_at_elego.de>
Date: Mon, 17 Aug 2009 15:42:57 +0100

On Mon, Aug 17, 2009 at 10:42:53AM +0800, Edmund Wong wrote:
> Hi,
>
> "One ping only"
> - Capt. Ramius.
> HfRO
>
> Is there something missing from this patch? Is the log
> ok?
>
> Any help appreciated.

> [[[
>
> Moved the EOL functions from the
> libsvn_subr/subst.c library to libsvn_subr/eol.c
> and from include/svn_subst.h to
> include/private/svn_eol_private.h. This is to
> allow future consolidation of EOL functions to
> be placed in the new files instead of being
> placed all over the place.

You can use fewer line breaks there. Wrapping log message at about
72 chars is good.

> Changed patch.c, subst-tests.c and diff-file.c
> to reflect the change in libraries for
> svn_subst_find_eol_start(), svn_subst_detect_eol(),
> and svn_subst_detect_file_eol().
>
> Updated the build.conf to include the new private
> header (svn_eol_private.h).
>
> * build.conf:
> (msvc-export) : Added private\svn_eol_private.h to end of list.

No space before colon.

>
> * subversion/libsvn_diff/diff-file.c:
> Include private/svn_eol_private.h"
>
> * subversion/libsvn_subr/eol.c
> New.

Can be on one line:

* subversion/libsvn_subr/eol.c: New.

>
> * subversion/include/private/svn_eol_private.h
> New.
>
> * subversion/libsvn_subr/subst.c:
> (svn_subst_find_eol_start,
> svn_subst_detect_eol,
> svn_subst_detect_file_eol): Moved ...
>
> * subversion/libsvn_subr/eol.c
> (svn_subst_find_eol_start,
> svn_subst_detect_eol,
> svn_subst_detect_file_eol): ...to here.

I think we want to rename the functions to svn_eol_* as well.

> * subversion/include/svn_subst.h
> (svn_subst_find_eol_start,
> svn_subst_detect_eol,
> svn_subst_detect_file_eol): Move ...
>
> * subversion/include/private/svn_eol_private.h
> (svn_subst_find_eol_start,
> svn_subst_detect_eol,
> svn_subst_detect_file_eol): ... to here.

You could collapse the above four entries into two:

* subversion/libsvn_subr/subst.c,
  subversion/include/svn_subst.h
  (svn_subst_find_eol_start, svn_subst_detect_eol,
   svn_subst_detect_file_eol): Moved from here ...

* subversion/libsvn_subr/eol.c,
  subversion/include/private/svn_eol_private.h
  (svn_subst_find_eol_start,
   svn_subst_detect_eol,
   svn_subst_detect_file_eol): ... to here.

>
> * subversion/libsvn_client/patch.c
> Included private/svn_eol_private.h
>
> * subversion/tests/libsvn_subr/subst-test.c
> Included private/svn_eol_private.h.
>
> Patch by Edmund Wong <ed {at} kdtc.net>
> Suggested by: stsp
>

> Index: subversion/include/private/svn_eol_private.h
> ===================================================================
> --- subversion/include/private/svn_eol_private.h (revision 0)
> +++ subversion/include/private/svn_eol_private.h (revision 0)
> @@ -0,0 +1,86 @@
> +/**
> + * @copyright
> + * ====================================================================
> + * Licensed to the Subversion Corporation (SVN Corp.) under one
> + * or more contributor license agreements. See the NOTICE file
> + * distributed with this work for additional information
> + * regarding copyright ownership. The SVN Corp. licenses this file
> + * to you under the Apache License, Version 2.0 (the
> + * "License"); you may not use this file except in compliance
> + * with the License. You may obtain a copy of the License at
> + *
> + * http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing,
> + * software distributed under the License is distributed on an
> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + * KIND, either express or implied. See the License for the
> + * specific language governing permissions and limitations
> + * under the License.
> + * ====================================================================
> + * @endcopyright
> + *
> + * @file svn_auth_private.h
> + * @brief Subversion's authentication system - Internal routines

You need to adjust these two lines above.

Rest looks great!

Thanks,
Stefan

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2384389
Received on 2009-08-17 16:44:11 CEST

This is an archived mail posted to the Subversion Dev mailing list.