Skip to content

add port number support#3292

Merged
msivasubramaniaan merged 8 commits intoredhat-developer:mainfrom
msivasubramaniaan:2781-add-support-for-devfile-port-and-name-detection
Sep 23, 2023
Merged

add port number support#3292
msivasubramaniaan merged 8 commits intoredhat-developer:mainfrom
msivasubramaniaan:2781-add-support-for-devfile-port-and-name-detection

Conversation

@msivasubramaniaan
Copy link
Copy Markdown
Collaborator

Fixes: #2781

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 20, 2023

Codecov Report

Patch coverage: 18.75% and project coverage change: -0.89% ⚠️

Comparison is base (b1ef719) 37.14% compared to head (1e3b137) 36.25%.
Report is 1 commits behind head on main.

❗ Current head 1e3b137 differs from pull request most recent head a23c956. Consider uploading reports for the commit a23c956 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3292      +/-   ##
==========================================
- Coverage   37.14%   36.25%   -0.89%     
==========================================
  Files          77       77              
  Lines        5333     5359      +26     
  Branches     1024     1033       +9     
==========================================
- Hits         1981     1943      -38     
- Misses       3352     3416      +64     
Files Changed Coverage Δ
src/odo/componentType.ts 62.50% <ø> (ø)
.../webview/create-component/createComponentLoader.ts 10.81% <0.00%> (-0.31%) ⬇️
src/webview/common-ext/createComponentHelpers.ts 14.45% <10.00%> (-0.41%) ⬇️
src/webview/devfile-registry/registryViewLoader.ts 25.86% <16.66%> (-1.17%) ⬇️
src/odo.ts 71.51% <20.00%> (-11.52%) ⬇️
src/openshift/nameValidator.ts 47.82% <50.00%> (ø)
src/odo/command.ts 64.80% <100.00%> (-3.50%) ⬇️

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread src/webview/common-ext/createComponentHelpers.ts Outdated
Comment thread src/webview/common/componentNameInput.tsx Outdated
Comment thread src/webview/common-ext/createComponentHelpers.ts
@msivasubramaniaan
Copy link
Copy Markdown
Collaborator Author

@datho7561 Thanks for your review. I just addressed the review comments and pushed the code. Can you please have a look on the latest code?

datho7561
datho7561 previously approved these changes Sep 20, 2023
Copy link
Copy Markdown
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

Looks good, and works well. Thanks, Muthu!

@datho7561
Copy link
Copy Markdown
Contributor

It seems like there are linter errors

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan
Copy link
Copy Markdown
Collaborator Author

It seems like there are linter errors

Fixed lint errors

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@mohitsuman
Copy link
Copy Markdown
Contributor

@msivasubramaniaan can you please share the gif/video of how it behaves.

@datho7561
Copy link
Copy Markdown
Contributor

Here's what it looks like:

Screenshot from 2023-09-21 08-53-28

@datho7561
Copy link
Copy Markdown
Contributor

Here's a small bug I found:

  • Create a component, then delete it's devfile and .odo so that it's not recognized as a component
  • Right click on the folder, then select "create component from folder"
  • Set the port to 9090, then click next
  • The port gets changed to 3000. I guess it's being auto detected from the node project?
  • Click "Create Component"
  • I get the error message "portNumber.trim is not a function":

Screenshot from 2023-09-21 08-59-41

@datho7561
Copy link
Copy Markdown
Contributor

The integration tests seem to be failing.

Signed-off-by: msivasubramaniaan <msivasub@redhat.com>
@msivasubramaniaan
Copy link
Copy Markdown
Collaborator Author

@mohitsuman Here is the demo gif:
port-number

@datho7561
Copy link
Copy Markdown
Contributor

datho7561 commented Sep 22, 2023

Currently, you can't set the port for components created from template or imported from git

It's actually only in the "from template" case. I think it's okay to not show the option there, since the port is also set in the sample code, so it's misleading to set the port in the devfile but provide the code with the old port.

Copy link
Copy Markdown
Contributor

@datho7561 datho7561 left a comment

Choose a reason for hiding this comment

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

Looks good! Feel free to merge after rebasing

@msivasubramaniaan msivasubramaniaan merged commit 760a1c0 into redhat-developer:main Sep 23, 2023
@msivasubramaniaan msivasubramaniaan deleted the 2781-add-support-for-devfile-port-and-name-detection branch September 23, 2023 05:25
@msivasubramaniaan
Copy link
Copy Markdown
Collaborator Author

@datho7561 Thanks for your review.

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.

Add support for devfile, port and name detection

3 participants