remoting/host/constants.h - Issue 10535082: /C++ readability/ - Make Chromoting Host report crashes to Breakpad (Windows only). The user must e…

Side by Side Diff: remoting/host/constants.h

Issue 10535082: /C++ readability/ - Make Chromoting Host report crashes to Breakpad (Windows only). The user must e… (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: Created 8 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch | Annotate | Revision Log

OLD	NEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #ifndef REMOTING_HOST_CONSTANTS_H_	5 #ifndef REMOTING_HOST_CONSTANTS_H_

6 #define REMOTING_HOST_CONSTANTS_H_	6 #define REMOTING_HOST_CONSTANTS_H_

7	7

	8 #include "base/string16.h"
	Peter Kasting 2012/06/14 20:16:27 Nit: #ifdef this as well. I suggest moving the bl Nit: #ifdef this as well. I suggest moving the block below up to here so you can use a single #if for both. alexeypa (please no reviews) 2012/06/15 18:45:49 I got rid of string16 entirely. BTW I don't thin Show quoted text On 2012/06/14 20:16:27, Peter Kasting wrote: > Nit: #ifdef this as well. I suggest moving the block below up to here so you > can use a single #if for both. I got rid of string16 entirely. BTW I don't think I could move this include below. The guide says that platform specific includes should go right after regular includes: "For platform-specific #includes, put them below the "regular" includes, separated with a blank line." Peter Kasting 2012/06/15 19:02:42 Yes, I was suggesting the opposite (to move some c Show quoted text On 2012/06/15 18:45:49, alexeypa wrote: > On 2012/06/14 20:16:27, Peter Kasting wrote: > > Nit: #ifdef this as well. I suggest moving the block below up to here so you > > can use a single #if for both. > > I got rid of string16 entirely. > > BTW I don't think I could move this include below. The guide says that platform > specific includes should go right after regular includes: > > "For platform-specific #includes, put them below the "regular" includes, > separated with a blank line." Yes, I was suggesting the opposite (to move some code below up to here rather than to move this #include downwards) as you're correct that we don't want #includes in the middle of the file. Sorry if I was unclear. (And it's moot now anyway.) alexeypa (please no reviews) 2012/06/15 19:51:24 Well, actually your comment was quite clear. I jus Show quoted text On 2012/06/15 19:02:42, Peter Kasting wrote: > Sorry if I was unclear. Well, actually your comment was quite clear. I just misread it.
	9

8 namespace remoting {	10 namespace remoting {

9	11

10 // Known host exit codes.	12 // Known host exit codes.

11 // Please keep this enum in sync with:	13 // Please keep this enum in sync with:

12 // remoting/host/installer/mac/PrivilegedHelperTools/	14 // remoting/host/installer/mac/PrivilegedHelperTools/

13 // org.chromium.chromoting.me2me.sh	15 // org.chromium.chromoting.me2me.sh

14 enum HostExitCodes {	16 enum HostExitCodes {

15 kSuccessExitCode = 0,	17 kSuccessExitCode = 0,

16 kReservedForX11ExitCode = 1,	18 kReservedForX11ExitCode = 1,

17 kInvalidHostConfigurationExitCode = 2,	19 kInvalidHostConfigurationExitCode = 2,

18 kInvalidHostIdExitCode = 3,	20 kInvalidHostIdExitCode = 3,

19 kInvalidOauthCredentialsExitCode = 4,	21 kInvalidOauthCredentialsExitCode = 4,

20	22

21 // The range of the exit codes that should be interpreted as a permanent error	23 // The range of the exit codes that should be interpreted as a permanent error

22 // condition.	24 // condition.

23 kMinPermanentErrorExitCode = kInvalidHostConfigurationExitCode,	25 kMinPermanentErrorExitCode = kInvalidHostConfigurationExitCode,

24 kMaxPermanentErrorExitCode = kInvalidOauthCredentialsExitCode	26 kMaxPermanentErrorExitCode = kInvalidOauthCredentialsExitCode

25 };	27 };

26	28

	29 #if defined(OS_WIN)

	30
	Peter Kasting 2012/06/14 20:16:27 Nit: Extra blank line here and below. There aren' Nit: Extra blank line here and below. There aren't definitive rules on this, but the style guide examples generally place blank lines inside most namespace definitions, and not inside most preprocessor directive blocks. alexeypa (please no reviews) 2012/06/15 18:45:49 Done. Show quoted text On 2012/06/14 20:16:27, Peter Kasting wrote: > Nit: Extra blank line here and below. There aren't definitive rules on this, > but the style guide examples generally place blank lines inside most namespace > definitions, and not inside most preprocessor directive blocks. Done.
	31 // The Omaha Appid of the host.

	32 extern const char16 kHostOmahaAppid[];
	Peter Kasting 2012/06/14 20:16:27 This should be a wchar_t rather than a string16, g This should be a wchar_t rather than a string16, given how it will be used in breakpad_win.cc. alexeypa (please no reviews) 2012/06/15 18:45:49 Done. Show quoted text On 2012/06/14 20:16:27, Peter Kasting wrote: > This should be a wchar_t rather than a string16, given how it will be used in > breakpad_win.cc. Done.
	33

	34 #endif // defined(OS_WIN)

	35

27 } // namespace remoting	36 } // namespace remoting

28	37

29 #endif // REMOTING_HOST_CONSTANTS_H_	38 #endif // REMOTING_HOST_CONSTANTS_H_

OLD	NEW

« remoting/host/breakpad_win.cc ('K') | « remoting/host/breakpad_win.cc ('k') | remoting/host/constants_win.cc » ('j') | remoting/host/constants_win.cc » ('J')