Hi Goetz, I did not aim to sort the includes in the attachListener_<os>.cpp files. I also just saw that I made a change in attachListener_aix.cpp for some other merge-related reasons - but it was not correctly sorted before and neither is after 😉 This has to be done in a future change...
Best regards Christoph > -----Original Message----- > From: Lindenmaier, Goetz > Sent: Mittwoch, 11. April 2018 09:56 > To: Langer, Christoph <christoph.lan...@sap.com>; serviceability- > d...@openjdk.java.net > Cc: hotspot-...@openjdk.java.net > Subject: RE: RFR (M): 8201247: Various cleanups in the attach framework > > Hi Christoph, > > I'm familiar with the non-system includes in hotspot, > there mostly a total alphabetical ordering is followed, like in > http://hg.openjdk.java.net/jdk/hs/file/0d8ed8b2ac4f/src/hotspot/cpu/x86/ > c1_CodeStubs_x86.cpp > > It is. However, I have put "subdirs" first. That is, the includes from sys/* > Also, that's not true, see the first file in your webrev, which is sorted as I > would expect: > http://cr.openjdk.java.net/~clanger/webrevs/8201247.0/src/hotspot/os/aix > /attachListener_aix.cpp.html > while this sorts as you state: > http://cr.openjdk.java.net/~clanger/webrevs/8201247.0/src/jdk.attach/linux > /native/libattach/VirtualMachineImpl.c.html > > Is there a different pattern followed in hotspot and jdk coding? > > In case you resort them (have fun :) ), no new webrev is needed. > > Best regards, > Goetz. > > > > -----Original Message----- > > From: Langer, Christoph > > Sent: Mittwoch, 11. April 2018 09:45 > > To: Lindenmaier, Goetz <goetz.lindenma...@sap.com>; serviceability- > > d...@openjdk.java.net > > Cc: hotspot-...@openjdk.java.net > > Subject: RE: RFR (M): 8201247: Various cleanups in the attach framework > > > > Hi Goetz, > > > > thanks for the review. > > > > > You say you are sorting the includes, but in the VirtualMachineImpl.c > > > files the order is changed, but according to which order? It's > > > not alphabetical as in other files. > > > > It is. However, I have put "subdirs" first. That is, the includes from sys/* > > come first (in alphabetical order). > > > > Best regards > > Christoph