create AuthorityOverridingTransportFactory instance only in need

[skyao]

in class AuthorityOverridingTransportFactory, we do nothing just to override the authority, and if authorityOverride is null, in fact we really do nothing in AuthorityOverridingTransportFactory.

So I suggest that we should add a null check before we new AuthorityOverridingTransportFactory instance. If authorityOverride is null, we don't need to create it, we can just use the original one returned by buildTransportFactory().

[grpc-kokoro]

Thanks for your pull request. The automated tests will run as soon as one of the admins verifies this change is ok to test.

[ejona86]

So this is an optimization?

Could you make authorityOverride required-non-null in AuthorityOverridingTransportFactory and remove the condition from it?

[ejona86]

The CI is failing with a style violation.

[skyao]

Yes, it is a small optimization.

I agree with your suggestion:

1. make authorityOverride required-non-null in AuthorityOverridingTransportFactory
2. remove the condition from it

I will do these and also fix the style violation.
Thank you !

[skyao]

codecov/project — 83.33% (-0.03%) compared to 511bea0 at 83.36%

I should write unit test case to increase the code coverage before you accept this pull request ?

[buchgr]

@skyao we love tests! :-). Out of curiosity, why is this "optimization" relevant?

[ejona86]

@skyao the codecov/project failure is fine; we can't yet define a tolerance, so the reduction isn't probably based on your change. And if we look at codecov/patch you've covered all the lines you changed.

[ejona86]

This LGTM. Could someone else do a review?

[carl-mastrangelo]

wrap this line so that all the args on the second line.

[carl-mastrangelo]

A few nits.

[skyao]

All comments had been accepted and code is updated to above comments, except the one that use static import for Preconditons.checkNotNull(). This is due to the fact that in this class there are some other Proconditions check but all of them don't use static import. I think we should keep the same style: use static or not for all the statements.

Anyway, if you insist that for this one we should use static import, I will follow.

[carl-mastrangelo]

@skyao LGTM, please squash your commits and I can merge.

[ejona86]

Following the convention of the file with checkNotNull is appropriate. Thanks!

Note that when we line wrap we wrap 4 spaces, not 2. I've discovered that our checkstyle is not working for this case, apparently because it is too old. There are multiple style violations that the newer version detects, so there will be some cleanup anyway later.