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 export objcutil.go API #49

Closed
wants to merge 6 commits into from
Closed

Conversation

cfergeau
Copy link
Contributor

At the moment, users of Code-Hex/vz can use the helper API (NSObject, NSError,
NSArray, pointer.Ptr(), pointer.Releases()) which was added for wrapping objc
in go.
This API should not be used outside of vz implementation, and trying to
interact with it could even make vz misbehave.
This PR makes these helpers private to the vz package.

Users of the vz package don't need to be exposed to these low-level
details of the objc/go interface.
The type is renamed from NSArray to nsArray.
After this commit, NSArray will no longer be listed at
https://pkg.go.dev/github.com/Code-Hex/vz/v2#NSArray
Users of the vz package don't need to be exposed to these low-level
details of the objc/go interface.
The type is renamed from NSObject to nsObject.
After this commit, NSObject will no longer be listed at
https://pkg.go.dev/github.com/Code-Hex/vz/v2#NSObject
Users of the vz package don't need to be exposed to these low-level
details of the objc/go interface.
The type is renamed from NSError to nsError.
After this commit, NSError will no longer be listed at
https://pkg.go.dev/github.com/Code-Hex/vz/v2#NSError
Most of the types listed at
https://pkg.go.dev/github.com/Code-Hex/vz/v2 currently expose a
Release() method because they embed a 'pointer' instance.
This method is only used internally for the go/objc interactions, and is
not useful to have in the exported API. This commit renames Release to
release so that it's no longer accessible by users of Code-Hex/vz
vz code currently has a lot of occurrences of this kind of constructs
when creating go wrappers for objc objects:
```
config := &VirtioSoundDeviceConfiguration{
    pointer: pointer{
        ptr: C.newVZVirtioSoundDeviceConfiguration(),
    },
}
```

This is a bit verbose, but most importantly this exposes the 'ptr'
field of the 'pointer' struct. In one of the next commits, I'd like to
rename this field, so as a preparatory commit, this introduces a
`newPointer` method to replace the code above with:
```
config := &VirtioSoundDeviceConfiguration{
    pointer: newPointer(C.newVZVirtioSoundDeviceConfiguration()),
}
```
which is shorter, and hides the `ptr` field from code outside of
objcutil.go.
Users of Code-Hex/vz  don't need access to the raw objc pointer, but
it's currently exposed as a Ptr() method.
This commit makes this method private (ptr()). This involves renaming
the `ptr` field to `wrappedPtr`, which is easy to after the addition of
newPointer().
This will remove all Ptr() methods from
https://pkg.go.dev/github.com/Code-Hex/vz/v2#BridgedNetworkDeviceAttachment.Ptr
@Code-Hex
Copy link
Owner

@cfergeau Thanks for creating a new PR!
However, this change is a very low priority for me. So I will check later

@cfergeau
Copy link
Contributor Author

However, this change is a very low priority for me. So I will check later

Yup, this is understandable :) Maybe it would be better to move objcutil to internal/objc instead of making its methods private.

@Code-Hex
Copy link
Owner

Following the Contribution Guideline, Once we've discussed what's wrong and what should be done about it the issue, we can fix it.

https://github.com/Code-Hex/vz/blob/master/CONTRIBUTING.md#did-you-write-a-patch-that-fixes-a-bug

@Code-Hex Code-Hex closed this Sep 23, 2022
Repository owner locked as too heated and limited conversation to collaborators Sep 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants