Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't compile in direct calls to callback functions #1

Merged
merged 1 commit into from
Oct 11, 2018

Conversation

GregMefford
Copy link
Member

When compiling a project that includes spandex_phoenix (in the example below, the spandex_example project), it throws a compiler warning because we're compiling-in a direct function call on a module in the application using us as a library (which we obviously don't depend on from spandex_phoenix).

Using apply gets around this because it's dynamically finding and calling the function at runtime.

==> spandex_phoenix
Compiling 1 file (.ex)
warning: function PhoenixBackend.Tracer.finish_span/0 is undefined (module PhoenixBackend.Tracer is not available)
Found at 2 locations:
  lib/spandex_phoenix/instrumenter.ex:16
  lib/spandex_phoenix/instrumenter.ex:24

warning: function PhoenixBackend.Tracer.start_span/2 is undefined (module PhoenixBackend.Tracer is not available)
Found at 2 locations:
  lib/spandex_phoenix/instrumenter.ex:12
  lib/spandex_phoenix/instrumenter.ex:20

@GregMefford
Copy link
Member Author

Also, this would be a good learning opportunity for how I'm supposed for format my commit messages and stuff. 😅

@zachdaniel
Copy link
Member

For this one, something like:

fix: Indirectly call trace function to remove warning

More in depth description here optionally, but just the first line is fine.

If you can shorten the message enough, you could also use a scope like fix(Instrumenter): did the thing

@GregMefford GregMefford merged commit 9928a8d into master Oct 11, 2018
@GregMefford GregMefford deleted the compiler_warning branch October 11, 2018 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants