Hi Nils,

On 11/05/2012 11:45 PM, Nils Loodin wrote:
And, to be clear, I updated a few lines after the conversation with
David. Here's the lastest:
http://cr.openjdk.java.net/~nloodin/7165755/webrev.03
<http://cr.openjdk.java.net/~nloodin/7165755/webrev.0>

I'm okay with this change. Do you have a second reviewer for this final format?

Also, David, could you sponsor my push if you're happy?

Where are you pushing to? hotspot-rt?

I can do the push once my home directory server comes back online :(

And once I see a second reviewer.

Thanks,
David


Dear regards,
Nils Loodin

On May 11, 2012, at 14:07 , Nils Loodin wrote:

Lists accidentally dropped of here.

For the official record, Davd Holmes, are you hereby happy with the
state of my changes and ready to stand by that as an official reviewer? :)

Regards,
Nils Loodin

Begin forwarded message:

*From: *David Holmes <[email protected]
<mailto:[email protected]>>
*Subject: **Re: RFR: 7165755 OS Information much longer on linux than
other platforms*
*Date: *May 11, 2012 12:52:12 GMT+02:00
*To: *Nils Loodin <[email protected]
<mailto:[email protected]>>

On 11/05/2012 5:22 PM, Nils Loodin wrote:
Missed one:

39 #include "os_linux.hpp"

Gah. Indeed.


Do we want the initial st->print("OS:") on the brief info the way we
have on the full info?
I judged no, due to the fact that this would (well in our case anyway,
but I thought generally) be used by other tools to get a brief info.
They would then have that string as a label in a gui, or something else.
Or if they want to print somewhere, they should print that string on
their own.

Ok.

About OS_xx.cpp including OS_xx.hpp, I feel I'm missing something.. can
you please help me say why that's a no-no? And what does it have to do
with the old included implementation?

It isn't that it is wrong, it's that it hasn't been necessary. So I
was wondering why it was now necessary, as if it had been necessary I
would have expected it to be done when the old include system got
converted. I think these files only need include the generic os.hpp
and that it turn will include the platform specific ones.

Changing the above, are you ok with the changes?

Yes.

Though we should probably be having this conversation on the open
lists ;-)

Cheers,
David



David

Regards,
Nils Loodin





Reply via email to