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

Follow-up to comments on #6654 (support for autoBuild and deployByDefault) #6720

Merged
merged 7 commits into from
Apr 7, 2023
Merged

Follow-up to comments on #6654 (support for autoBuild and deployByDefault) #6720

merged 7 commits into from
Apr 7, 2023

Conversation

rm3l
Copy link
Member

@rm3l rm3l commented Apr 5, 2023 •
edited
Loading

What type of PR is this:
/kind code-refactoring

What does this PR do / why we need it:
This is a follow-up PR to #6654 (support for autoBuild and deployByDefault fields), addressing some of the comments left there.

Which issue(s) this PR fixes:
Related to #5694

PR acceptance criteria:

How to test changes / Special notes to the reviewer:

rm3l added 5 commits April 5, 2023 11:08
@rm3l
Remove additional non-nil checks on component fields since they are a…
9544a55 
…lready filtered by type

We are confident that the fields we expect
(comp.{Image,Kubernetes,OpenShift}) are non-nil.
@rm3l
Rename 'libdevfile.GetImageComponentsToPush' into 'libdevfile.GetImag…
1a29def 
…eComponentsToPushAutomatically'
@rm3l
Document with examples cases where 'deployByDefault' and 'autoBuild' …
9060596 
…are `false` and the component is not referenced
@rm3l
Rename 'image-autobuild-true-and-referenced' command into 'image-auto…
65812b8 
…build-true' in test Devfile
@rm3l
Simplify parsing the output on Podman in the tests when looking for K…
a85e969 
…8s/OpenShift components
@rm3l rm3l added this to the v3.10.0 🚀 milestone Apr 5, 2023
@netlify
Copy link

netlify bot commented Apr 5, 2023 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit 4bdcec6
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/642ecc00b522c6000811d124
😎 Deploy Preview https://deploy-preview-6720--odo-docusaurus-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@openshift-ci openshift-ci bot requested review from feloy and kadel April 5, 2023 15:50
@rm3l rm3l requested review from valaparthvi and removed request for kadel April 5, 2023 15:51
@odo-robot
Copy link

odo-robot bot commented Apr 5, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Apr 5, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Apr 5, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Apr 5, 2023 •
edited
Loading

Windows Tests (OCP) on commit b2d429d finished with errors.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Apr 5, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Apr 5, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Apr 5, 2023 •
edited
Loading

Kubernetes Docs Tests on commit c82583b finished successfully.
View logs: TXT HTML

@rm3l rm3l closed this Apr 6, 2023
@rm3l rm3l reopened this Apr 6, 2023
valaparthvi
valaparthvi suggested changes Apr 6, 2023
View reviewed changes
Copy link
Contributor

@valaparthvi valaparthvi left a comment

Choose a reason for hiding this comment

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

Thank you for following up on these!

tests/examples/source/devfiles/nodejs/devfile-autobuild-deploybydefault.yaml Outdated Show resolved Hide resolved
@rm3l
Revert "Rename 'image-autobuild-true-and-referenced' command into 'im…
4bdcec6 
…age-autobuild-true' in test Devfile"

This reverts commit 65812b8.
@rm3l rm3l requested a review from valaparthvi April 6, 2023 13:41
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Apr 6, 2023
@feloy
Copy link
Contributor

feloy commented Apr 6, 2023 

 [FAILED] [12.944 seconds]
odo dev debug command tests when starting with Devfile with autoBuild or deployByDefault components when running odo dev with some components not referenced in the Devfile [BeforeEach] should create the appropriate resources
  [BeforeEach] /go/odo_1/tests/integration/cmd_dev_debug_test.go:429
  [It] /go/odo_1/tests/integration/cmd_dev_debug_test.go:459
[...]
  [odo]  ✗  failed to create Kubernetes resources associated with the component: pods "ocp-deploybydefault-true-and-not-referenced" is forbidden: error looking up service account cmd-dev-debug-test459zaf/default: serviceaccount "default" not found

@feloy feloy closed this Apr 6, 2023
@feloy feloy reopened this Apr 6, 2023
@rm3l rm3l closed this Apr 7, 2023
@rm3l rm3l reopened this Apr 7, 2023
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 7, 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 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@feloy
Copy link
Contributor

feloy commented Apr 7, 2023

/override windows-integration-test/Windows-test

Flaky e2e test

[FAILED] [206.998 seconds]
E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
C:/Users/Administrator.ANSIBLE-TEST-VS/3652/tests/e2escenarios/e2e_test.go:337

@openshift-ci
Copy link

openshift-ci bot commented Apr 7, 2023

@feloy: Overrode contexts on behalf of feloy: windows-integration-test/Windows-test

In response to this:

/override windows-integration-test/Windows-test

Flaky e2e test

[FAILED] [206.998 seconds]
E2E Test starting with non-empty Directory add Binding [It] should verify developer workflow of using binding as env in innerloop
C:/Users/Administrator.ANSIBLE-TEST-VS/3652/tests/e2escenarios/e2e_test.go:337

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-merge-robot openshift-merge-robot merged commit c2d850f into redhat-developer:main Apr 7, 2023
@rm3l rm3l deleted the 5694_follow_up_after_support_for_autobuild_deploybydefault branch April 7, 2023 07:35
@rm3l rm3l added the area/refactoring Issues or PRs related to code refactoring label Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valaparthvi valaparthvi valaparthvi approved these changes

@feloy feloy Awaiting requested review from feloy

Assignees

@valaparthvi valaparthvi

Labels
area/refactoring Issues or PRs related to code refactoring lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
v3.10.0 🚀
Development

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