PelionIoT / mbed-trace

mbed trace library
Apache License 2.0
18 stars 15 forks source link

Reworked header. #23

Closed tommikas closed 8 years ago

tommikas commented 8 years ago

Function declarations always exist, but when tracing is disabled they are hidden by the dummy macros.

tommikas commented 8 years ago

@kjbracey-arm Is this about what you had in mind?

jupe commented 8 years ago

seems that you have to write some more tests because CI fails:

11:36:56 Code coverage enforcement failed for the following metrics:
11:36:56     Lines's stability is 65.41 and set mininum stability is 83.77.
11:36:56     Conditionals's stability is 50.56 and set mininum stability is 64.08
tommikas commented 8 years ago

Ah yes, that was actually something I did want comments on: Any other usage macros/aliases to add?

tommikas commented 8 years ago

Hm... I don't think the code coverage failure is because of this change. ARMmbed/mbed-trace#20 is failing for that too, no?

Still, regardless of what changes cause the failure, those tests should be added.

tommikas commented 8 years ago

Regarding the tr_array macro, I'm not sure it would add much. It isn't really possible to assign any standard values to the arguments of tracearray, like in the case of the other tr macros, so it would simply function as another alias for mbed_trace_array, saving 3 characters per line when compared to trace_array.

kjbracey commented 8 years ago

This is basically what I was thinking of, yes. Like it, just those few comments.

tommikas commented 8 years ago

Re-evaluated for each inclusion of the header - can include it more than once in a compilation unit.

Yeah, realized this after pushing, heh.

FEA_TRACE_SUPPORT going to go in via the separate commit, and then resolving the merge conflict?

That's what I was thinking.

tommikas commented 8 years ago

@kjbracey-arm. Incorporated your previous comments. Anything else come to mind?

jupe commented 8 years ago

lgtm

tommikas commented 8 years ago

test this please

SeppoTakalo commented 8 years ago

+1

jupe commented 8 years ago

lgtm