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

feat: core.ToContext(ctx, v) for ctx initialization #852

Merged
merged 5 commits into from
Apr 4, 2021

Conversation

rurirei
Copy link
Contributor

@rurirei rurirei commented Apr 3, 2021

related #831 #841

this brings a method to check and create Instance for ctx, while may someone needs this. (like me ::)

@kslr kslr requested a review from xiaokangwang April 3, 2021 13:52
@rurirei rurirei marked this pull request as ready for review April 4, 2021 00:37
@Loyalsoldier Loyalsoldier merged commit aa40b8b into v2fly:master Apr 4, 2021
@xiaokangwang
Copy link
Contributor

xiaokangwang commented Apr 5, 2021

Hi, I have to revert some of your change: you have created an exported API, which is an unsupported usage and enable so many other unsupported usages.

You shouldn't use

//go:linkname mustToContextForced github.com/v2fly/v2ray-core/v4.mustToContext
func mustToContextForced(ctx context.Context, v *Instance) context.Context

to forcefully export this function into your package to create a context suitable for interact with V2Ray's internal component.
This kind of usage is unsupported and may break at any time.

@rurirei
Copy link
Contributor Author

rurirei commented Apr 5, 2021

@xiaokangwang ok. i don't know how to design a func MustToContext (i see there is a func MustFromContext as i created a similar one), and i wait for your opinion on this pr (sorry for no extra comment for this). you can do your way kindly, thanks.

@rurirei rurirei deleted the context0 branch April 6, 2021 04:24
@xiaokangwang
Copy link
Contributor

The reason I did so is that V2Ray have so many internal components that may change behavior at any time, you shouldn't depend on them remain unchanged. You could develop software that interact with them, but it may stop working any time.

@rurirei
Copy link
Contributor Author

rurirei commented May 3, 2021

@xiaokangwang thanks for your explaination.

maybe there is a purpose to make a public ToContext(ctx, v) for go test.

ctx := context.WithValue(context.Background(), v2rayKey, v)

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.

4 participants