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

Implement devfile state in odo api #6941

Merged
merged 45 commits into from
Jul 5, 2023
Merged

Implement devfile state in odo api #6941

merged 45 commits into from
Jul 5, 2023

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Jun 29, 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 #6903

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

The changes can be tested by:

  • starting odo with ODO_EXPERIMENTAL_MODE=t odo dev --platform podman --api-server from a directory containing a Devfile
  • Starting the ui with cd ui && ng serve
  • Manually accessing http://localhost:4200 (paste a Devfile into the first page, ...)
  • running e2e tests with cd ui && npm run cypress:run

@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 Jun 29, 2023
@netlify
Copy link

netlify bot commented Jun 29, 2023 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit cdacd69
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/64a52d8385223e0007985eca

@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 Jun 29, 2023
@openshift-ci openshift-ci bot requested review from anandrkskd and kadel June 29, 2023 09:08
@odo-robot
Copy link

odo-robot bot commented Jun 29, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jun 29, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jun 29, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jun 29, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jun 29, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jun 29, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jun 29, 2023 •
edited
Loading

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

@odo-robot
Copy link

odo-robot bot commented Jun 29, 2023 •
edited
Loading

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

@feloy feloy force-pushed the feature-6903/ui-devfile-state branch from ac9ab1e to 8af202b Compare June 30, 2023 08:17
@feloy feloy changed the title [wip] Implement devfile state in odo api Implement devfile state in odo api Jul 3, 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 Jul 3, 2023
@feloy feloy requested review from rm3l and removed request for kadel and anandrkskd July 3, 2023 11:28
@feloy feloy force-pushed the feature-6903/ui-devfile-state branch from ee92b4d to a9bd13e Compare July 3, 2023 13:46
rm3l
rm3l reviewed Jul 4, 2023
View reviewed changes
ododevapispec.yaml Outdated Show resolved Hide resolved
ododevapispec.yaml Outdated Show resolved Hide resolved
ododevapispec.yaml Outdated Show resolved Hide resolved
ododevapispec.yaml Show resolved Hide resolved
ododevapispec.yaml Outdated Show resolved Hide resolved
ododevapispec.yaml Show resolved Hide resolved
rm3l
rm3l reviewed Jul 4, 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.

Another minor point: can you update ui/Makefile and ui/devfile.yaml to remove references to the WebAssembly module?

@feloy feloy requested a review from rm3l July 4, 2023 14:50
@feloy feloy force-pushed the feature-6903/ui-devfile-state branch from f7a4adb to 68d5bb7 Compare July 4, 2023 15:37
rm3l
rm3l reviewed Jul 5, 2023
View reviewed changes
Makefile
@@ -246,3 +246,13 @@ generate-apiserver: ## Generate OpenAPISpec library based on ododevapispec.yaml
--additional-properties=outputAsLibrary=true,onlyInterfaces=true,hideGenerationTimestamp=true && \
echo "Formatting generated files:" && go fmt ./pkg/apiserver-gen/... && \
echo "Removing pkg/apiserver-gen/api/openapi.yaml" && rm ./pkg/apiserver-gen/api/openapi.yaml

.PHONY: generate-apifront
generate-apifront: ## Generate OpenAPISpec library based on ododevapispec.yaml inside ui/src/app
Copy link
Member

Choose a reason for hiding this comment

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

Could we also have a single Make target that would generate both the server and the client codes?

rm3l
rm3l reviewed Jul 5, 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.

Also, I was curious as to why the ui-e2e.yaml GitHub Workflow did not run, as I was expecting it to not pass at this point..
Can you check why it did not run?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 5, 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 8 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@feloy
Copy link
Contributor Author

feloy commented Jul 5, 2023

Also, I was curious as to why the ui-e2e.yaml GitHub Workflow did not run, as I was expecting it to not pass at this point.. Can you check why it did not run?

Good catch, there was an error on the paths description. It is running now. Thanks

rm3l
rm3l approved these changes Jul 5, 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.

Thanks for your work on this!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 5, 2023
@rm3l
Copy link
Member

rm3l commented Jul 5, 2023

/override cypress-run

The E2E job will be handled once we have #6943 implemented.

@openshift-ci
Copy link

openshift-ci bot commented Jul 5, 2023

@rm3l: Overrode contexts on behalf of rm3l: cypress-run

In response to this:

/override cypress-run

The E2E job will be handled once we have #6943 implemented.

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 649181c into redhat-developer:main Jul 5, 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

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.

Move the odo dev web UI state to odo
3 participants
@feloy @rm3l @openshift-merge-robot