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

perf: optimize dockerfile #1298

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

thebedigupta
Copy link
Contributor

@thebedigupta thebedigupta commented Oct 10, 2024

Copy link

changeset-bot bot commented Oct 10, 2024

⚠️ No Changeset found

Latest commit: 0420a04

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@thebedigupta thebedigupta changed the title improve generate function structure perf: optimize dockerfile Oct 10, 2024
@@ -8,14 +6,17 @@ WORKDIR /app
ENV PUPPETEER_EXECUTABLE_PATH /usr/bin/chromium-browser
ENV PUPPETEER_SKIP_CHROMIUM_DOWNLOAD true

# Include the ARG instruction in this build stage
Copy link
Member

Choose a reason for hiding this comment

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

please include one line comment explaining why it should be included in build stage

# Install dependencies and AsyncAPI Generator in a single RUN command
RUN apk --no-cache add git chromium && \
rm -rf /var/cache/apk/* && \
npm install -g --ignore-scripts "@asyncapi/generator@${ASYNCAPI_GENERATOR_VERSION}"
Copy link
Member

Choose a reason for hiding this comment

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

sonar was just suggesting to surround only the variable name with double quotes

Copy link
Member

@derberg derberg left a comment

Choose a reason for hiding this comment

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

once you react to all comments, please also run build of the docker image locally + use the docker to verify all works as expected

@thebedigupta
Copy link
Contributor Author

once you react to all comments, please also run build of the docker image locally + use the docker to verify all works as expected

I am updating test results file Please review it and then just tell me if I did something wrong

@derberg
Copy link
Member

derberg commented Oct 21, 2024

I see in test result that you did run docker build but haven't seen docker run.

use the docker to verify all works as expected

I mean't - please try to use the image you build locally, to check if generation still works

going with this should do the trick

@thebedigupta
Copy link
Contributor Author

I see in test result that you did run docker build but haven't seen docker run.

use the docker to verify all works as expected

I mean't - please try to use the image you build locally, to check if generation still works

going with this should do the trick

@derberg I was trying to understand where things goes wrong Give me some time if you have any suggestions on how to run image then please let me know
docker

Copy link

sonarcloud bot commented Oct 23, 2024

Copy link
Member

derberg commented Oct 24, 2024

but the command that is under the link I provided, isn't it working?

@thebedigupta
Copy link
Contributor Author

but the command that is under the link I provided, isn't it working?

Update the test results Please check again

@derberg
Copy link
Member

derberg commented Oct 28, 2024

@thebedigupta you mean in description? I don't see info in test result that docker run was called

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.

3 participants