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.

Sign up for GitHub

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

Jump to bottom

Set platform and platformVersion in telemetry #6540

Merged
merged 9 commits into from
Jan 28, 2023
Merged

Set platform and platformVersion in telemetry #6540

merged 9 commits into from
Jan 28, 2023

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Jan 25, 2023 •
edited
Loading

What type of PR is this:

/kind feature

What does this PR do / why we need it:

Which issue(s) this PR fixes:

Fixes partially #6508

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Jan 25, 2023
@openshift-ci openshift-ci bot added the kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation label Jan 25, 2023
@netlify
Copy link

netlify bot commented Jan 25, 2023 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit 41fba62
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63d4187de797d50009498907

@feloy feloy requested a review from rm3l January 25, 2023 15:21
@openshift-ci openshift-ci bot requested a review from kadel January 25, 2023 15:22
@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

OpenShift Tests on commit 3ceeb42 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

NoCluster Tests on commit 3ceeb42 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

Validate Tests on commit 3ceeb42 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

Kubernetes Tests on commit 3ceeb42 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 25, 2023 •
edited
Loading

Windows Tests (OCP) on commit 3ceeb42 finished successfully.
View logs: TXT HTML

@feloy feloy changed the title [WIP] Set platform and platformVersion in telemetry Set platform and platformVersion in telemetry Jan 26, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. Required by Prow. label Jan 26, 2023
@feloy feloy force-pushed the feature-6508/telemetry-platform branch from a2ea475 to 7c96bfe Compare January 26, 2023 07:11
@odo-robot
Copy link

odo-robot bot commented Jan 26, 2023 •
edited
Loading

Unit Tests on commit 3ceeb42 finished successfully.
View logs: TXT HTML

rm3l
rm3l reviewed Jan 26, 2023
View reviewed changes
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

Can you also update the USAGE_DATA.md file to mention these new properties (along with the "experimental" property) ?

@feloy feloy force-pushed the feature-6508/telemetry-platform branch from e3140e7 to 744052f Compare January 26, 2023 11:07
@feloy feloy requested a review from rm3l January 26, 2023 12:11
@feloy feloy closed this Jan 26, 2023
@feloy feloy reopened this Jan 26, 2023
rm3l
rm3l requested changes Jan 26, 2023
View reviewed changes
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

LGTM feature-wise. Added a few comments related to the tests.

tests/integration/cmd_dev_test.go Outdated Show resolved Hide resolved
tests/integration/cmd_dev_test.go Outdated Show resolved Hide resolved
tests/integration/cmd_dev_test.go Outdated Show resolved Hide resolved
tests/integration/cmd_devfile_deploy_test.go Outdated Show resolved Hide resolved
tests/integration/cmd_devfile_deploy_test.go Outdated Show resolved Hide resolved
rm3l
rm3l reviewed Jan 26, 2023
View reviewed changes
Copy link
Member

@rm3l rm3l left a comment

Choose a reason for hiding this comment

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

One last point. Can you also update the should record the telemetry data correctly test in cmd_devfile_init_test.go to make sure that the telemetry data does not contain the new platform and platformVersion fields?

@feloy feloy requested a review from rm3l January 27, 2023 09:24
@feloy feloy force-pushed the feature-6508/telemetry-platform branch from 69bf086 to b360b54 Compare January 27, 2023 10:29
rm3l
rm3l approved these changes Jan 27, 2023
View reviewed changes
Copy link
Member

@rm3l rm3l left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM - thanks.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jan 27, 2023
@feloy
Copy link
Contributor Author

feloy commented Jan 27, 2023

/override OpenShift-Integration-tests/OpenShift-Integration-tests
Flaky e2e test

/override windows-integration-test/Windows-test
Flaky logs test

@openshift-ci
Copy link

openshift-ci bot commented Jan 27, 2023

@feloy: Overrode contexts on behalf of feloy: OpenShift-Integration-tests/OpenShift-Integration-tests, windows-integration-test/Windows-test

In response to this:

/override OpenShift-Integration-tests/OpenShift-Integration-tests
Flaky e2e test

/override windows-integration-test/Windows-test
Flaky logs test

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jan 27, 2023
@feloy feloy requested a review from rm3l January 27, 2023 14:34
@feloy
Copy link
Contributor Author

feloy commented Jan 27, 2023

Puhsed a new commit to fix version in some OpenShift clusters (including the one used by Prow)

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jan 27, 2023
@feloy feloy closed this Jan 27, 2023
@feloy feloy reopened this Jan 27, 2023
@feloy feloy force-pushed the feature-6508/telemetry-platform branch from afac86d to 41fba62 Compare January 27, 2023 18:31
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jan 27, 2023
@openshift-ci
Copy link

openshift-ci bot commented Jan 27, 2023

New changes are detected. LGTM label has been removed.

@feloy
Copy link
Contributor Author

feloy commented Jan 27, 2023

Rebased

@odo-robot
Copy link

odo-robot bot commented Jan 27, 2023 •
edited
Loading

OpenShift Unauthenticated Tests on commit 3ceeb42 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 27, 2023 •
edited
Loading

Kubernetes Docs Tests on commit 3ceeb42 finished with errors.
View logs: TXT HTML

@rm3l rm3l closed this Jan 28, 2023
@rm3l rm3l reopened this Jan 28, 2023
@sonarcloud
Copy link

sonarcloud bot commented Jan 28, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@feloy
Copy link
Contributor Author

feloy commented Jan 28, 2023

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests
Fixed by #6545

@openshift-ci
Copy link

openshift-ci bot commented Jan 28, 2023

@feloy: Overrode contexts on behalf of feloy: Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests

In response to this:

/override Kubernetes-Integration-Tests/Kubernetes-Docs-Integration-Tests
Fixed by #6545

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@feloy
Copy link
Contributor Author

feloy commented Jan 28, 2023

/override ci/prow/v4.11-integration-e2e

@openshift-ci
Copy link

openshift-ci bot commented Jan 28, 2023

@feloy: Overrode contexts on behalf of feloy: ci/prow/v4.11-integration-e2e

In response to this:

/override ci/prow/v4.11-integration-e2e

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@rm3l rm3l added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jan 28, 2023
@openshift-merge-robot openshift-merge-robot merged commit 0bce606 into redhat-developer:main Jan 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rm3l rm3l rm3l approved these changes

@valaparthvi valaparthvi Awaiting requested review from valaparthvi

@kadel kadel Awaiting requested review from kadel

Assignees

@rm3l rm3l

Labels
kind/feature Categorizes issue as a feature request. For PRs, that means that the PR is the implementation lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@feloy @rm3l @openshift-merge-robot