Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-12 Thread Ivan Zhakov
On 5 January 2015 at 15:43, Stefan Fuhrmann
stefan.fuhrm...@wandisco.com wrote:
 On Fri, Jan 2, 2015 at 2:24 PM, Ivan Zhakov i...@visualsvn.com wrote:

 On 2 January 2015 at 15:43,  stef...@apache.org wrote:
  Author: stefan2
  Date: Fri Jan  2 12:43:42 2015
  New Revision: 1649012
 
  URL: http://svn.apache.org/r1649012
  Log:
  Merge branches/fsx-id in /trunk, remove the BRANCH-README and
  resolve trivial conflicts.
 Stefan,

 Do I understand correctly that you have merged branch without
 resolving concerns about dirent_t structure naming raised dev@ list
 two weeks ago [1] ?

 [1] http://svn.haxx.se/dev/archive-2014-12/0130.shtml


 You do. Since there was more than just the dirent_t
 structure that should have been renamed, the changes
 were beyond the scope of the branch. Moreover, fewer
 merge conflicts could be expected by first merging
 outstanding changes into FSX and then modifying
 hundreds of LOC.

 So, I merged first and went through all non-ID-related
 changes later the same day. All nice and consistent
 now with moderate work. Makes sense?

Ok, that makes sense.

While it will be easier for reviewer if naming issue was fixed before
branch merge. But merge conflicts is argument to do this after merge.


-- 
Ivan Zhakov


Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-05 Thread Stefan Fuhrmann
On Fri, Jan 2, 2015 at 2:24 PM, Ivan Zhakov i...@visualsvn.com wrote:

 On 2 January 2015 at 15:43,  stef...@apache.org wrote:
  Author: stefan2
  Date: Fri Jan  2 12:43:42 2015
  New Revision: 1649012
 
  URL: http://svn.apache.org/r1649012
  Log:
  Merge branches/fsx-id in /trunk, remove the BRANCH-README and
  resolve trivial conflicts.
 Stefan,

 Do I understand correctly that you have merged branch without
 resolving concerns about dirent_t structure naming raised dev@ list
 two weeks ago [1] ?

 [1] http://svn.haxx.se/dev/archive-2014-12/0130.shtml


You do. Since there was more than just the dirent_t
structure that should have been renamed, the changes
were beyond the scope of the branch. Moreover, fewer
merge conflicts could be expected by first merging
outstanding changes into FSX and then modifying
hundreds of LOC.

So, I merged first and went through all non-ID-related
changes later the same day. All nice and consistent
now with moderate work. Makes sense?

-- Stefan^2.


Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-02 Thread Philip Martin
Ivan Zhakov i...@visualsvn.com writes:

 On 2 January 2015 at 18:59, Philip Martin philip.mar...@wandisco.com wrote:
 Ivan Zhakov i...@visualsvn.com writes:

 Do I understand correctly that you have merged branch without
 resolving concerns about dirent_t structure naming raised dev@ list
 two weeks ago [1] ?

 The names appear to have been changed (dirent_t to svn_fs_x__dirent_t)
 as part of the merge commit.

 What commit do you mean?

 I see the following in this diff email:
 [[[
 @@ -2411,8 +2407,8 @@ sorted(apr_array_header_t *entries)
  static int
  compare_dirents(const void *a, const void *b)
  {
 -  const svn_fs_dirent_t *lhs = *((const svn_fs_dirent_t * const *) a);
 -  const svn_fs_dirent_t *rhs = *((const svn_fs_dirent_t * const *) b);
 +  const dirent_t *lhs = *((const dirent_t * const *) a);
 +  const dirent_t *rhs = *((const dirent_t * const *) b);

return strcmp(lhs-name, rhs-name);
  }
 ]]]

My mistake, I was looking at libsvn_fs_fs/fs.h in my tree and saw the
rename, but it occurred later in r1649062.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*


Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-02 Thread Ivan Zhakov
On 2 January 2015 at 15:43,  stef...@apache.org wrote:
 Author: stefan2
 Date: Fri Jan  2 12:43:42 2015
 New Revision: 1649012

 URL: http://svn.apache.org/r1649012
 Log:
 Merge branches/fsx-id in /trunk, remove the BRANCH-README and
 resolve trivial conflicts.
Stefan,

Do I understand correctly that you have merged branch without
resolving concerns about dirent_t structure naming raised dev@ list
two weeks ago [1] ?

[1] http://svn.haxx.se/dev/archive-2014-12/0130.shtml

-- 
Ivan Zhakov


Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-02 Thread Philip Martin
Ivan Zhakov i...@visualsvn.com writes:

 Do I understand correctly that you have merged branch without
 resolving concerns about dirent_t structure naming raised dev@ list
 two weeks ago [1] ?

The names appear to have been changed (dirent_t to svn_fs_x__dirent_t)
as part of the merge commit.

-- 
Philip Martin | Subversion Committer
WANdisco // *Non-Stop Data*


Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-02 Thread Ivan Zhakov
On 2 January 2015 at 18:59, Philip Martin philip.mar...@wandisco.com wrote:
 Ivan Zhakov i...@visualsvn.com writes:

 Do I understand correctly that you have merged branch without
 resolving concerns about dirent_t structure naming raised dev@ list
 two weeks ago [1] ?

 The names appear to have been changed (dirent_t to svn_fs_x__dirent_t)
 as part of the merge commit.

What commit do you mean?

I see the following in this diff email:
[[[
@@ -2411,8 +2407,8 @@ sorted(apr_array_header_t *entries)
 static int
 compare_dirents(const void *a, const void *b)
 {
-  const svn_fs_dirent_t *lhs = *((const svn_fs_dirent_t * const *) a);
-  const svn_fs_dirent_t *rhs = *((const svn_fs_dirent_t * const *) b);
+  const dirent_t *lhs = *((const dirent_t * const *) a);
+  const dirent_t *rhs = *((const dirent_t * const *) b);

   return strcmp(lhs-name, rhs-name);
 }
]]]


-- 
Ivan Zhakov


Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-02 Thread Philip Martin
Philip Martin philip.mar...@wandisco.com writes:

 My mistake, I was looking at libsvn_fs_fs/fs.h in my tree and saw the
 rename, but it occurred later in r1649062.

Should be looking at libsvn_fs_x/fs.h

-- 
Philip


Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-02 Thread Branko Čibej
On 02.01.2015 14:24, Ivan Zhakov wrote:
 On 2 January 2015 at 15:43,  stef...@apache.org wrote:
 Author: stefan2
 Date: Fri Jan  2 12:43:42 2015
 New Revision: 1649012

 URL: http://svn.apache.org/r1649012
 Log:
 Merge branches/fsx-id in /trunk, remove the BRANCH-README and
 resolve trivial conflicts.
 Stefan,

 Do I understand correctly that you have merged branch without
 resolving concerns about dirent_t structure naming raised dev@ list
 two weeks ago [1] ?

 [1] http://svn.haxx.se/dev/archive-2014-12/0130.shtml

Stefan already said he'd change the names; the purpose of the naming
convention thread was basically to determine what to change them /to/.
He doesn't have to commit the struct renames the minute you point out
naming issues, regardless of other considerations.

You might consider not breathing down peoples' necks on what is, when
all is said and done, a volunteer effort.

-- Brane


Re: svn commit: r1649012 [1/3] - in /subversion/trunk: ./ subversion/libsvn_fs_x/

2015-01-02 Thread Branko Čibej
On 02.01.2015 14:24, Ivan Zhakov wrote:
 On 2 January 2015 at 15:43,  stef...@apache.org wrote:
 Author: stefan2
 Date: Fri Jan  2 12:43:42 2015
 New Revision: 1649012

 URL: http://svn.apache.org/r1649012
 Log:
 Merge branches/fsx-id in /trunk, remove the BRANCH-README and
 resolve trivial conflicts.
 Stefan,

 Do I understand correctly that you have merged branch without
 resolving concerns about dirent_t structure naming raised dev@ list
 two weeks ago [1] ?

 [1] http://svn.haxx.se/dev/archive-2014-12/0130.shtml

Stefan already said he'd change the names; the purpose of the naming
convention thread was basically to determine what to change them /to/.
He doesn't have to commit the struct renames the minute you point out
naming issues, regardless of other considerations.

You might consider not breathing down peoples' necks on what is, when
all is said and done, a volunteer effort.

-- Brane