-
Notifications
You must be signed in to change notification settings - Fork 21
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
Migrate all codegen to weaver #227
Comments
For reference, .NET semconv "package" is here: https://github.com/open-telemetry/opentelemetry-dotnet-contrib/tree/main/src/OpenTelemetry.SemanticConventions |
For Node.js/JS/TS open-telemetry/opentelemetry-js#4690 will be migrating semconv codegen to use weaver. |
PHP migration completed and can be checked off... |
@open-telemetry/weaver-maintainers Please take a look at the opentelemetry-cpp migration to weaver: Looking for any feedback or suggestions on the weaver / jinja code in particular. |
Very nice use of Weaver. On your side, do you see any areas for improvement in Weaver that would make things easier for you? |
Yes, I have a few suggestions. When a developer find a weaver.yaml file, of a jinja file, and sees it the first time, If I find a C++ or java file, I know where to find doc about the C++ or java library, language syntax, etc. If I find weaver.yaml, I can only stare at the content and be stuck. Do not assume people will know of, or even have heard of, weaver and jinja, and even less that they will know where to find doc about it. This is why I added systematically pointers to the DOC, in comments, in these files, for opentelemetry-cpp. I think it will be beneficial to add that in the examples and templates. By pointers to the doc, I mean something like this:
|
Also, systematically adding a comment in the generated code, to indicate:
should be strongly suggested as a best practice, in the jinja templates. Not all SIG are doing this, from what I could find. |
The text was updated successfully, but these errors were encountered: