[Clean up] Introduce a BLUETOOTH_EXPORT macro to control symbol visibility outside of //device/blue…

[Clean up] Introduce a BLUETOOTH_EXPORT macro to control symbol visibility outside of //device/bluetooth.

BUG=423116
TEST=none
R=armansito@chromium.org

[armansito, 2014-10-15 19:10:30]

On 2014/10/15 00:48:10, Ilya Sherman wrote:

Looks like there are some compile errors while linking device_unittests.

[Ilya Sherman, 2014-10-16 00:50:27]

On 2014/10/15 19:10:30, armansito wrote:
> On 2014/10/15 00:48:10, Ilya Sherman wrote:
> 
> Looks like there are some compile errors while linking device_unittests.

Hmm, true enough.  I'm not sure what the way to address that is.  I could
annotate the tested classes with the BLUETOOTH_EXPORT_PRIVATE macro, but the
"_PRIVATE" suffix is just documentation -- it makes the classes visible outside
of the component either way.  Do you think it's worth making that change?  If
not, do you have an alternative suggestion, or should we abandon this change?

[Ilya Sherman, 2014-10-16 00:51:12]

On 2014/10/16 00:50:27, Ilya Sherman wrote:
> On 2014/10/15 19:10:30, armansito wrote:
> > On 2014/10/15 00:48:10, Ilya Sherman wrote:
> > 
> > Looks like there are some compile errors while linking device_unittests.
> 
> Hmm, true enough.  I'm not sure what the way to address that is.  I could
> annotate the tested classes with the BLUETOOTH_EXPORT_PRIVATE macro, but the
> "_PRIVATE" suffix is just documentation -- it makes the classes visible
outside
> of the component either way.  Do you think it's worth making that change?  If
> not, do you have an alternative suggestion, or should we abandon this change?

Actually, I suppose I could define BLUETOOTH_IMPLEMENTATION for the unit tests. 
Does that sound reasonable?

[Ilya Sherman, 2014-10-16 00:58:58]

On 2014/10/16 00:51:12, Ilya Sherman wrote:
> On 2014/10/16 00:50:27, Ilya Sherman wrote:
> > On 2014/10/15 19:10:30, armansito wrote:
> > > On 2014/10/15 00:48:10, Ilya Sherman wrote:
> > > 
> > > Looks like there are some compile errors while linking device_unittests.
> > 
> > Hmm, true enough.  I'm not sure what the way to address that is.  I could
> > annotate the tested classes with the BLUETOOTH_EXPORT_PRIVATE macro, but the
> > "_PRIVATE" suffix is just documentation -- it makes the classes visible
> outside
> > of the component either way.  Do you think it's worth making that change? 
If
> > not, do you have an alternative suggestion, or should we abandon this
change?
> 
> Actually, I suppose I could define BLUETOOTH_IMPLEMENTATION for the unit
tests. 
> Does that sound reasonable?

Sorry, I am clearly not pausing long enough between messages: The tests all live
under the device_unittests target defined within //device/device_tests.gyp.  So,
I could define BLUETOOTH_IMPLEMENTATION for all of the device unit tests, even
though that includes non-Bluetooth tests.  Does that sound okay?  There are a
fairly limited number of tests defined in that target, so it might be a good
compromise.

[armansito, 2014-10-16 21:57:28]

On 2014/10/16 00:58:58, Ilya Sherman wrote:
> On 2014/10/16 00:51:12, Ilya Sherman wrote:
> > On 2014/10/16 00:50:27, Ilya Sherman wrote:
> > > On 2014/10/15 19:10:30, armansito wrote:
> > > > On 2014/10/15 00:48:10, Ilya Sherman wrote:
> > > > 
> > > > Looks like there are some compile errors while linking device_unittests.
> > > 
> > > Hmm, true enough.  I'm not sure what the way to address that is.  I could
> > > annotate the tested classes with the BLUETOOTH_EXPORT_PRIVATE macro, but
the
> > > "_PRIVATE" suffix is just documentation -- it makes the classes visible
> > outside
> > > of the component either way.  Do you think it's worth making that change? 
> If
> > > not, do you have an alternative suggestion, or should we abandon this
> change?
> > 
> > Actually, I suppose I could define BLUETOOTH_IMPLEMENTATION for the unit
> tests. 
> > Does that sound reasonable?
> 
> Sorry, I am clearly not pausing long enough between messages: The tests all
live
> under the device_unittests target defined within //device/device_tests.gyp. 
So,
> I could define BLUETOOTH_IMPLEMENTATION for all of the device unit tests, even
> though that includes non-Bluetooth tests.  Does that sound okay?  There are a
> fairly limited number of tests defined in that target, so it might be a good
> compromise.

Are you suggesting that non-bluetooth tests have the BLUETOOTH_IMPLEMENTATION
macro on them? Sorry I'm just a bit confused. 

Now that I think about it maybe having Bluetooth tests as part of
device_unittests is a mistake in the first place, especially if we're turning
Bluetooth into a stand-alone component. So perhaps we will want bluetooth to
eventually live in components/bluetooth. For now, would it be possible to
compile the bluetooth unit tests from within bluetooth.gyp/gn and
include/execute it from the top-level device_unittests?

Anyway, in response to your original question, I would check with the top-level
device/ OWNERS. The majority of test cases may belong to bluetooth but there is
enough additional non-bluetooth stuff in there that I would check with them to
make sure what we're doing makes sense.