codegen: change static func names to avoid conflicts #86
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jagobagascon
force-pushed
the
feature/change-static-method-name-strategy-to-avoid-conflicts
branch
from
f8ff369
to
6ca4178
Compare
glerchundi
approved these changes
Mar 20, 2024
There was a problem hiding this comment.
The extra complexity added to keep backward compat and not knowing until when we should keep those one they land into the main branch inclines me to be much more drastic and allow ourselves to directly apply breaking changes. These kind of cases should be rare and quite easy to adapt once they bump from one version to another. So I would do the following:
- Add a note in the README about breaking changes and protect ourselves by explicitly allowing breaking changes although we know that there shouldn't be much more cases. I would use this PR as an example.
- Get rid of the extra complexity and just add a prefix to the types and regenerate those that are needed. We can also help tinygo/bluetooth by sending a bump and required changes.
WDYT?
This commit changes the naming strategy to include the class name as a prefix of the static function names. Static functions were generated as public functions using the method name found in the windows metadata. Since all static functions of a namespace are added to the same Go package, this could cause collisions when two classes had the same static method (`FromBluetoothAddress` for example can be found in both Bluetoothdevice and BluetoothLEDevice). fixes #85
jagobagascon
force-pushed
the
feature/change-static-method-name-strategy-to-avoid-conflicts
branch
from
6ca4178
to
22784f4
Compare
|
Considering that the change only involves renaming a few functions and the simpler code, I agree |
jagobagascon
deleted the
feature/change-static-method-name-strategy-to-avoid-conflicts
branch
This was referenced Mar 21, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Static functions were generated as public functions using the method name found in the windows metadata. Since all static functions of a namespace are added to the same Go package, this could cause collisions when two classes had the same static method (
FromBluetoothAddressfor example can be found in both Bluetoothdevice and BluetoothLEDevice).This commit changes the naming strategy to include the class name as a prefix of the static function names. And the existing methods will be kept for a while marked as deprecated to maintain backwards compatibility.
fixes #85