Docs: Update compatibility matrix for php #1904
Merged
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success1097 tests passing in 180 suites. Report generated by 🧪jest coverage report action from 04b5e02 |
|
@RikudouSage I don't think we have a plan to add segments here at the moment, the way they were designed is that segments are mapped to constraints on the backend unless the backend receives the client spec header and that header is greater than v4.2.0. So they should just work out the box on any SDK that supports constraints. The work on global segments is an optimisation for users that may have very, very large segments, and that's mostly to reduce overhead due to packet size and parsing the response |
sighphyre
reviewed
Aug 10, 2022
| @@ -98,7 +98,7 @@ If you see an item marked with a ❌ that you would find useful, feel free to re | |||
| | Can disable metrics | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | |||
| | Client registration | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | |||
| | Basic usage metrics (yes/no) | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | ✅ | | |||
| | [Impression data](../advanced/impression-data.md) | ⭕ | ⭕ | ⭕ | ⭕ | ⭕ | ⭕ | ⭕ | ⭕ | N/A | | |||
| | [Impression data](../advanced/impression-data.md) | ⭕ | ⭕ | ⭕ | ⭕ | ⭕ | ⭕ | ✅ | ⭕ | N/A | | |||
There was a problem hiding this comment.
I believe this was added to the node SDK as well, by this PR: Unleash/unleash-client-node#293
@thomasheartman Can you sanity check me here?
Suggested change
| | [Impression data](../advanced/impression-data.md) | ⭕ | ⭕ | ⭕ | ⭕ | ⭕ | ⭕ | ✅ | ⭕ | N/A | | |
| | [Impression data](../advanced/impression-data.md) | ⭕ | ✅ | ⭕ | ⭕ | ⭕ | ⭕ | ✅ | ⭕ | N/A | |
There was a problem hiding this comment.
I think it would be out of scope, so maybe creating an issue to update it or another PR?
There was a problem hiding this comment.
@sighphyre From what I can tell, the PR only adds it as a field that they expect to get from the server. It doesn't look like they do anything with it, so I don't think the node client supports impression data. I might be wrong, though.
Regardless, I agree with @RikudouSage that that should be a different PR in that case.
There was a problem hiding this comment.
Fair enough! Consider it ignored
sighphyre
reviewed
Aug 10, 2022
There was a problem hiding this comment.
Looks good to me, but I'd like @thomasheartman to do the final approval here, since he's our resident Word Smith
thomasheartman
requested changes
Aug 10, 2022
| @@ -3,7 +3,7 @@ title: Impression data | |||
| --- | |||
|
|
|||
| :::info Availability | |||
| The impression data feature was introduced in **Unleash 4.7**. Listening for events requires [an SDK that supports impression data events](../sdks/index.md#server-side-sdk-compatibility-table). Currently, it's only supported in the [Unleash Proxy client](../sdks/proxy-javascript.md) and [React Proxy client](../sdks/proxy-react.md). | |||
| The impression data feature was introduced in **Unleash 4.7**. Listening for events requires [an SDK that supports impression data events](../sdks/index.md#server-side-sdk-compatibility-table). | |||
There was a problem hiding this comment.
Good catch! Because we still don't have a compatibility table for frontend clients, I think we should still mention which frontend SDKs support it here. Something like this, perhaps?
Suggested change
| The impression data feature was introduced in **Unleash 4.7**. Listening for events requires [an SDK that supports impression data events](../sdks/index.md#server-side-sdk-compatibility-table). | |
| The impression data feature was introduced in **Unleash 4.7**. Listening for events requires [an SDK that supports impression data events](../sdks/index.md#server-side-sdk-compatibility-table). In addition to server-side SDKs that support it, it's also supported in the JavaScript-based proxy clients. |
There was a problem hiding this comment.
I reworded it a little bit, let me know if it's ok.
| @@ -79,7 +79,7 @@ You can enable impression data via the impression data toggle in the admin UI's | |||
| ## Example setup | |||
|
|
|||
| :::caution | |||
| This functionality is currently only supported in the [Unleash Proxy client](../sdks/proxy-javascript.md) and [React Proxy client](../sdks/proxy-react.md). | |||
| This functionality is currently only supported in some of our SDKs, please see the [compatibility table for SDKs](../sdks/index.md#server-side-sdk-compatibility-table). | |||
There was a problem hiding this comment.
Same thing as above: I think we should keep mention of the proxy clients in the note.
Suggested change
| This functionality is currently only supported in some of our SDKs, please see the [compatibility table for SDKs](../sdks/index.md#server-side-sdk-compatibility-table). | |
| This functionality is currently only supported the JavaScript-based proxy clients and some of some of the server-side clients. Please refer to the [compatibility table for SDKs](../sdks/index.md#server-side-sdk-compatibility-table). |
thomasheartman
approved these changes
Aug 10, 2022
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.
About the changes
Updated the compatibility table for php,
namePrefixandtagsfilters and IP CIDR syntax were added some time ago, impression data just today (not released yet but will be during today, more specifically after Unleash/unleash-client-php#104 gets merged).Discussion points