Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(219)

Issue 4631080: Version information for Syzygy (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
6 years, 3 months ago by chrisha
Modified:
6 years, 3 months ago
Reviewers:
Roger McFarlane, Siggi
CC:
sawbuck-changes_googlegroups.com
Base URL:
http://sawbuck.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Version information for Syzygy Added version information to Syzygy, with integration into the build system. Placed the version header in a top level project, 'common', with associated unittests, etc. Committed: http://code.google.com/p/sawbuck/source/browse/#svn/trunk369

Patch Set 1 : '' #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+326 lines, -0 lines) Patch
A syzygy/VERSION View 1 chunk +21 lines, -0 lines 0 comments Download
A syzygy/common/common.gyp View 1 chunk +107 lines, -0 lines 2 comments Download
A syzygy/common/common_unittests_main.cc View 1 chunk +25 lines, -0 lines 0 comments Download
A syzygy/common/syzygy_version.h View 1 chunk +64 lines, -0 lines 2 comments Download
A syzygy/common/syzygy_version.cc View 1 chunk +44 lines, -0 lines 2 comments Download
A syzygy/common/syzygy_version_unittest.cc View 1 chunk +35 lines, -0 lines 0 comments Download
A syzygy/common/version.gen.template View 1 chunk +28 lines, -0 lines 0 comments Download
M syzygy/syzygy.gyp View 1 chunk +1 line, -0 lines 0 comments Download
M syzygy/unittests.gypi View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 4
chrisha
PTAL.
6 years, 3 months ago (2011-06-30 19:36:05 UTC) #1
Roger McFarlane
lgtm with minor nits. http://codereview.appspot.com/4631080/diff/20/syzygy/common/syzygy_version.cc File syzygy/common/syzygy_version.cc (right): http://codereview.appspot.com/4631080/diff/20/syzygy/common/syzygy_version.cc#newcode35 syzygy/common/syzygy_version.cc:35: last_change_(last_change) { DCHECK(last_change != NULL); ...
6 years, 3 months ago (2011-06-30 19:45:09 UTC) #2
Siggi
lgtm http://codereview.appspot.com/4631080/diff/20/syzygy/common/common.gyp File syzygy/common/common.gyp (right): http://codereview.appspot.com/4631080/diff/20/syzygy/common/common.gyp#newcode34 syzygy/common/common.gyp:34: { nit: This looks very similar to what ...
6 years, 3 months ago (2011-06-30 19:49:52 UTC) #3
chrisha
6 years, 3 months ago (2011-06-30 19:51:34 UTC) #4
Thanks, committing.

http://codereview.appspot.com/4631080/diff/20/syzygy/common/common.gyp
File syzygy/common/common.gyp (right):

http://codereview.appspot.com/4631080/diff/20/syzygy/common/common.gyp#newcode34
syzygy/common/common.gyp:34: {
On 2011/06/30 19:49:52, Siggi wrote:
> nit: This looks very similar to what we do in sawbuck. When you reuse content
> from another file, you can make the diffs smaller by using svn cp to copy the
> originating file, then modify from there.

Yeah, I used the sawbuck file as a template, but I didn't think to SVN copy it.
I'll try to remember for next time!

http://codereview.appspot.com/4631080/diff/20/syzygy/common/syzygy_version.cc
File syzygy/common/syzygy_version.cc (right):

http://codereview.appspot.com/4631080/diff/20/syzygy/common/syzygy_version.cc...
syzygy/common/syzygy_version.cc:35: last_change_(last_change) {
On 2011/06/30 19:45:09, Roger McFarlane wrote:
> DCHECK(last_change != NULL);

Done.

http://codereview.appspot.com/4631080/diff/20/syzygy/common/syzygy_version.h
File syzygy/common/syzygy_version.h (right):

http://codereview.appspot.com/4631080/diff/20/syzygy/common/syzygy_version.h#...
syzygy/common/syzygy_version.h:41: template<class OutArchive> bool
Save(OutArchive* out_archive) const {
On 2011/06/30 19:45:09, Roger McFarlane wrote:
> DCHECK(out_archive != NULL);
> 
> similarly for in_archive below.

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted