-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Add App and SubApp resource methods #13425
base: main
Are you sure you want to change the base?
Conversation
I can see how this makes the code nicer, but it's more to maintain and synchronize and makes the docs much worse. I don't think we should do this.
I would happily approve a standalone PR for this though! |
The system param is |
@alice-i-cecile ok, I'll create a PR only for the non_send_resource (and maybe 👇 ?). Is there any other reasonable way to achieve the same result? Or, does it make sense to have some of the world methods but not all of them? I mean only those related to resources only, since they are handled inside plugins. @mockersf maybe the NonSend should be renamed to NonSendResource too? NonSend seems very generic (and also it's not strictly a noun 😊 ) |
Names should stay coherent, and if you update one you should update the others. When adding |
I see, it makes sense to keep names short. My 2c as a regular user: I found it confusing that regular resources are initialized as I wouldn't care about the longer name, both since I can always define my own internal type (or a macro) to shorten the name. Anyway, I'll create the PR, so that we can move the discussion there where it should belong 😊. |
I think this is a good compromise. I also had no clue that a |
|
Objective
Add helper methods to
App
andSubApp
to ease plugin maintainers and less verbose code. It also adds examples to show how to use these methods.It also deprecates the
World::contains_non_send
in favor of the more consistentWorld::contains_non_send_resource
.