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: report telemetry when sandbox login button pressed #91

Merged
merged 3 commits into from
Sep 1, 2023

Conversation

dgolovin
Copy link
Collaborator

Fix replaces url for a button + command to add telemetry tracking when button pressed

Signed-off-by: Denis Golovin [email protected]

@dgolovin dgolovin requested a review from a team as a code owner August 30, 2023 19:17
Signed-off-by: Denis Golovin <[email protected]>
Copy link
Collaborator

@benoitf benoitf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one problem that I see is that this new way of launching of command is only available in v1.4.0+ so it'll break on all the other versions of Podman Desktop

@dgolovin
Copy link
Collaborator Author

@benoitf Shouldn't we have engine compatibility check somewhere here probably? Then if extension requires newer engine version it should not be offered for update or it should indicate update is available, but not compatible with current podman desktop.

@benoitf
Copy link
Collaborator

benoitf commented Aug 31, 2023

@dgolovin until now it was not a priority until now but yes if we want to have newer versions not rolling out to old versions of Podman Desktop it would be a new enhancement.

but engine requirement should require probably to be first exposed at https://registry.podman-desktop.io/api/extensions.json

and I suppose in several other places when we grab "first version" from the list of versions.

coming back to the root problem of this PR

maybe we could check that if we have both href and command, command is used in the markdown implementation (in Podman Desktop)

so if we keep both href and command, old versions of Podman Desktop will call the href, and newer versions will call the command (and then telemetry will be reported)

@dgolovin
Copy link
Collaborator Author

@benoitf brilliant! it works, but I had to put href in double quotes to avoid parsing issued. Tested on v1.3.1 branch and main.

@dgolovin dgolovin merged commit 524a126 into redhat-developer:main Sep 1, 2023
4 checks passed
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.

2 participants