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: podman desktop extension #34

Merged
merged 9 commits into from
Dec 20, 2023
Merged

Conversation

feloy
Copy link
Contributor

@feloy feloy commented Dec 5, 2023

An image can be created and pushed with ./scripts/build.sh <image-name> <tag>

This creates a multi-platform image for (windows,linux,darwin) x (amd64,arm64).

Fixes #21

@lstocchi
Copy link
Contributor

lstocchi commented Dec 5, 2023

I built the extension, installed it on Desktop but i receive this message for every image.
image

@lstocchi
Copy link
Contributor

lstocchi commented Dec 5, 2023

Not sure if this is expected bc i have to do something or there is an error, so asking

@feloy
Copy link
Contributor Author

feloy commented Dec 5, 2023

Not sure if this is expected bc i have to do something or there is an error, so asking

Which OS/arch are you using?

@lstocchi
Copy link
Contributor

lstocchi commented Dec 5, 2023

windows amd64

@feloy
Copy link
Contributor Author

feloy commented Dec 5, 2023

Which version of Podman desktop are you using? Particularly, does it include this patch? podman-desktop/podman-desktop#5069

@lstocchi
Copy link
Contributor

lstocchi commented Dec 5, 2023

Which version of Podman desktop are you using? Particularly, does it include this patch? containers/podman-desktop#5069

i was using my local main branch but probably it was not rebased, let me re-try

@lstocchi
Copy link
Contributor

lstocchi commented Dec 5, 2023

Ok it was a problem with my branch. Works fine, thanks @feloy

Copy link
Contributor

@lstocchi lstocchi left a comment

Choose a reason for hiding this comment

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

LGTM

podman-desktop-extension/package.json Outdated Show resolved Hide resolved
Containerfile Outdated Show resolved Hide resolved
@feloy feloy requested a review from jeffmaury December 8, 2023 15:27
@feloy feloy force-pushed the desktop-extension branch from e129447 to 38bf64b Compare December 9, 2023 14:58
@feloy feloy requested a review from jeffmaury December 14, 2023 12:43
@feloy feloy force-pushed the desktop-extension branch 5 times, most recently from 222aa83 to 38bf64b Compare December 18, 2023 13:39
Copy link
Member

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

I can't test the extension: here is what I have when I run it with --extension-folder

image

I tested against main and v1.6.3 of Podman Desktop source code

"dependencies": {},
"devDependencies": {
"@podman-desktop/api": "next",
"@types/node": "^20.9.4",
Copy link
Member

Choose a reason for hiding this comment

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

Should be in phase with the node version use so ^18

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you trying to run by moving the sources into the podman desktop sources and building it as the same time as PD?

Or by downloading an image from the Extensions page?

Copy link
Member

Choose a reason for hiding this comment

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

I'm using with different code bases but from the Podman Dekstop one, I'm running yarn watch --extension-folder ../../redhat-developer/podman-desktop-image-checker-openshift-ext/podman-desktop-extension/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I didn't understand it was expected to be ran this way also.

I think I'll have to adapt the tsconfig.json file to make it compatible for both this way and built as an independent module into an image.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I've made a fix to make it buildable with yarn watch and inside an image.

But with yarn watch, you still won't have the doa executable built, and it won't be found by the extension. Do we want to automate this in some way?

@feloy feloy requested a review from jeffmaury December 19, 2023 09:22
Copy link
Member

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

When I try to build the image, I have a warning that yarn build but there is no lock file

Also would be good to add node_modules to .gitignore

@feloy
Copy link
Contributor Author

feloy commented Dec 20, 2023

When I try to build the image, I have a warning that yarn build but there is no lock file

Also would be good to add node_modules to .gitignore

It should be fixed with the latest commit

@feloy feloy requested a review from jeffmaury December 20, 2023 13:20
Copy link
Member

@jeffmaury jeffmaury left a comment

Choose a reason for hiding this comment

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

LGTM

@feloy feloy merged commit 40b0a79 into redhat-developer:main Dec 20, 2023
1 check 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.

Publish initial version of the extension
3 participants