-
Notifications
You must be signed in to change notification settings - Fork 9
watchers → enabled_watchers in getInfo #135
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
Conversation
This ensures it's clear which info is returned.
This is a PR for #131. |
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'll merge tomorrow if no-one else objects.
@bwoebi: What do you think about limiting extension to allow for future extension in the specification? |
Once we merge the separate virtual package for API, this isn't an issue. I do not see that future extension is exceedingly limited here. |
But those implementations have to release a new major version. IMO they should add a separate method if they want to add more data, that will have a way lower chance of conflicting. Additionally, this would make the return of |
Well, it's the choice of the implementations whether they choose to add more entries and incur the risk of having to sometime release a new major in the unlikely case of same key, different value. The same way round you may argue that you could - without this restriction - rename keys in the Driver without requiring the implementations to release a new major (because they're allowed to retain the old key then). |
No, you can't. This would break the API. I'm specifically talking about not breaking the consumer API, but still being able to break the SPI. |
@trowski @rdlowrey @WyriHaximus How do you think about the extension? |
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.
Makes perfect sense to me 👍
Could anyone merge this? |
This ensures it's clear which info is returned. I could have added another sentence to the docblock instead, but I think this makes it more obvious when consuming the code, too.
In general, I think that we should be careful with allowing extension here, as it limits us in adding more specified keys, such as
"disabled_watchers"
.