-
-
Notifications
You must be signed in to change notification settings - Fork 35
feat: import objects referenced using avro namespaces #246
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
feat: import objects referenced using avro namespaces #246
Conversation
stripPackageName(schemaName) { | ||
// If there is a dot in the schema name, it's probably an Avro schema with a fully qualified name (including the namespace.) | ||
const indexOfDot = schemaName.lastIndexOf('.'); | ||
if (indexOfDot > 0) { | ||
return { className: schemaName.substring(indexOfDot + 1), javaPackage: schemaName.substring(0, indexOfDot) }; | ||
} | ||
return { className: schemaName, javaPackage: undefined }; | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to utility class since it's common for all.js and applicationModel.js. Could also be used in other places in the future.
Not sure whether or not it should live in scsLib.js since the comment in the file says, // This contains functions that are common to both the all.js filter and the post-process.js hook
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My intention with scsLib.js was to have a place for code that could be shared between the filters (all.js) and any non-filters such as the pre and post hooks. So it would be fine to put that in scsLib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I'll move it there, thanks!
@@ -40,7 +40,7 @@ | |||
"@semantic-release/npm": "^7.1.3", | |||
"@semantic-release/release-notes-generator": "^9.0.1", | |||
"conventional-changelog-conventionalcommits": "^4.6.0", | |||
"eslint": "^6.8.0", | |||
"eslint": "^7.32.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not the most up-to-date version. I just wanted support for optional chaining so it isn't upset when I do ?.
which is cleaner than nested conditionals.
" | ||
|
||
import java.util.function.Consumer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some snapshots are fine, some have an extra space now. I can't be bothered to endlessly fiddle with spacing for every use case. One extra space sometimes is within my tolerance. Let me know if you think we must make the spacing perfect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no need to be picky here. It's better to make it easy to work with.
…d-stream-template into crushton/fix-243
Kudos, SonarCloud Quality Gate passed!
|
🎉 This PR is included in version 0.12.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Problem
When using the namespace field on an avro schema, the class will get put into the proper package, but the java package keyword wouldn't be used at the top of the class nor would it be imported in the generated application class.
Solution
When generating a schema, if there's a namespace, write that in as a package. When generating the application, look through the channels and find schemas that are going to be referenced. If they are in a different namespace, import them.
Design Notes
After adding the package name to the schema, all thats left is importing it in the main class. There are two ways to do this.
I chose option 2 because the code is cleaner and the variable states are going to make more sense because the class name is MyObject - not com.example.api.whatever.MyObject.
As for how we know what to import, we might be able to collect the package names when we build our function specs, but this would mean that we must build the function specs first, then retrieve the info either from a separate variable or a new parameter in the function spec. The import is too loosely related to a function spec, in my opinion, so I made a completely independent function that looks though the channels for the necessary things to import.
I still moved the function spec initialization earlier in the process though.
Related issue(s)
Resolves #243