Issue 8864: Updates README and the comments with information on how well various platforms are supported.
Patch Set 1 #Patch Set 2 : Addressed comments from Vlad and Chandler. #Patch Set 3 : Adds description on building gtest on one's own. #MessagesTotal messages: 9
I like it, with a small correction. Chandler, do you have anything to say? http://codereview.appspot.com/8864/diff/1/3 File README (right): http://codereview.appspot.com/8864/diff/1/3#newcode23 Line 23: also make our best effort to support other platforms (e.g. Solaris and In this case, even with their best effort, the amount of support the core members of the team are able to provide is uncomfortably close to zero. Maybe later when will have our cross-platform test infrastructure done, we'll set up builds and tests of Google Test on all those strange and beautiful OSes, but for now I propose to change the rest of this paragraph to he following: We rely on our external contributors to support Google Test on other platforms (such as Solaris and IBM z/OS) because core members of the Google Test team have no access to them. Bug reports and (especially!) patches for those platforms are very welcome at googletestframework@googlegroups.com.
Sign in to reply to this message.
http://codereview.appspot.com/8864/diff/1/3 File README (right): http://codereview.appspot.com/8864/diff/1/3#newcode19 Line 19: This blank line is inconsistent with this file's formatting. http://codereview.appspot.com/8864/diff/1/3#newcode23 Line 23: also make our best effort to support other platforms (e.g. Solaris and On 2008/11/15 02:10:20, Vlad wrote: > In this case, even with their best effort, the amount of support the core > members of the team are able to provide is uncomfortably close to zero. Maybe > later when will have our cross-platform test infrastructure done, we'll set up > builds and tests of Google Test on all those strange and beautiful OSes, but for > now I propose to change the rest of this paragraph to he following: > > We rely on our external contributors to support Google Test on other platforms > (such as Solaris and IBM z/OS) because core members of the Google Test team have > no access to them. Bug reports and (especially!) patches for those platforms are > very welcome at googletestframework@googlegroups.com. I think "best effort" and Zhanyong's disclaimer of access restrictions is pretty good. If someone says "the compiler on my weird machine says '...', what do I do?" we can certainly look at the code, and provide some guesses as to how they should debug. We also conform to standards that should be supported across tool chains. Sounds like "best effort" to me. ;] http://codereview.appspot.com/8864/diff/1/3#newcode25 Line 25: have no access to them, some versions of Google Test may not work well "some versions" is confusing here to me when reading as Joe the Coder. How about "Google Test may have outstanding issues on these platforms"? That makes it clear that we're hoping to address them, but haven't gotten there yet, and leads nicely into the next sentence. http://codereview.appspot.com/8864/diff/1/2 File include/gtest/internal/gtest-port.h (right): http://codereview.appspot.com/8864/diff/1/2#newcode79 Line 79: // most stable support. Since core members of the Google Test project Rather than the last 5 lines of this, why not refer back to the README after clarifying the four which have strong support?
Sign in to reply to this message.
I've uploaded a new snapshot. Another look? Thanks. http://codereview.appspot.com/8864/diff/1/3 File README (right): http://codereview.appspot.com/8864/diff/1/3#newcode19 Line 19: On 2008/11/15 02:17:17, chandlerc wrote: > This blank line is inconsistent with this file's formatting. Done. http://codereview.appspot.com/8864/diff/1/3#newcode23 Line 23: also make our best effort to support other platforms (e.g. Solaris and On 2008/11/15 02:10:20, Vlad wrote: > In this case, even with their best effort, the amount of support the core > members of the team are able to provide is uncomfortably close to zero. Maybe > later when will have our cross-platform test infrastructure done, we'll set up > builds and tests of Google Test on all those strange and beautiful OSes, but for > now I propose to change the rest of this paragraph to he following: > > We rely on our external contributors to support Google Test on other platforms > (such as Solaris and IBM z/OS) because core members of the Google Test team have > no access to them. Bug reports and (especially!) patches for those platforms are > very welcome at googletestframework@googlegroups.com. Obviously I'm biased, but here's the thinking: - "make our best effort" != "guaranteed to work", so we are not over-promosing to say that we'll make our beest effort. - Simply saying that we rely on external contributors to support other platforms can be misleading, as it gives people the impression that we don't care. http://codereview.appspot.com/8864/diff/1/3#newcode25 Line 25: have no access to them, some versions of Google Test may not work well On 2008/11/15 02:17:17, chandlerc wrote: > "some versions" is confusing here to me when reading as Joe the Coder. How about > "Google Test may have outstanding issues on these platforms"? That makes it > clear that we're hoping to address them, but haven't gotten there yet, and leads > nicely into the next sentence. Done. http://codereview.appspot.com/8864/diff/1/2 File include/gtest/internal/gtest-port.h (right): http://codereview.appspot.com/8864/diff/1/2#newcode79 Line 79: // most stable support. Since core members of the Google Test project On 2008/11/15 02:17:17, chandlerc wrote: > Rather than the last 5 lines of this, why not refer back to the README after > clarifying the four which have strong support? My rationale is that it's better user experience if the user doesn't have to open another file and find the right paragraph in it. Yes, there's some duplication of words, but I think it's not significant and well worth it. Do you still want me to change this?
Sign in to reply to this message.
I added a section on using one's own build system to build gtest. Please take a look at the latest snapshot. Thanks!
Sign in to reply to this message.
http://codereview.appspot.com/8864/diff/604/405 File README (right): http://codereview.appspot.com/8864/diff/604/405#newcode223 Line 223: You should also let our users know that they can use SCons to build the project and their own tests. We already have the SConscript; the users just have to provide their own SConstruct file.
Sign in to reply to this message.
http://codereview.appspot.com/8864/diff/604/405 File README (right): http://codereview.appspot.com/8864/diff/604/405#newcode223 Line 223: On 2008/11/17 21:42:07, Vlad wrote: > You should also let our users know that they can use SCons to build the project > and their own tests. We already have the SConscript; the users just have to > provide their own SConstruct file. Good idea. Could you let me know which platforms our SConscript supports?
Sign in to reply to this message.
http://codereview.appspot.com/8864/diff/604/405 File README (right): http://codereview.appspot.com/8864/diff/604/405#newcode223 Line 223: On 2008/11/17 21:54:00, Zhanyong wrote: > On 2008/11/17 21:42:07, Vlad wrote: > > You should also let our users know that they can use SCons to build the > project > > and their own tests. We already have the SConscript; the users just have to > > provide their own SConstruct file. > > Good idea. Could you let me know which platforms our SConscript supports? > Hrm. Come to think of it, the HEAD version in svn has Windows specific features. The patch at http://codereview.appspot.com/8654/diff/1/23 pulls in changes that make it cross platform. So it makes sense to mention SCons only after we commit that patch -- or a separate one with just that file (amended, of course). I withdraw my comment until then.
Sign in to reply to this message.
OK. So can I submit this, Vlad and Chandler?
Sign in to reply to this message.
|
