-
-
Notifications
You must be signed in to change notification settings - Fork 260
feat: asyncapi on alpine distros #1844
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
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: a9351cb The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Changeset has been generated for this PR as part of auto-changeset workflow.Please review the changeset before merging the PR.If you are a maintainer or the author of the PR, you can change the changeset by clicking here Tip If you don't want auto-changeset to run on this PR, you can add the label |
e9e4741 to
1c312c4
Compare
|
Hey @Gmin2 I really like this, I have a few suggestions/doubts if you don't mind.
|
|
@asyncapi/bounty_team |
b4b5e65 to
d0141a8
Compare
ec71a6c to
dc87fff
Compare
|
|
can i have a review on this , idk why the test is broken here? |
Upon viewing the changed files 2 files i found are in the PR but not required in my opinion:-
|
|
@Shurtu-gal can you take a look at this? thanks : ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Could you try running the Github action locally through docker image once? Reason I am asking this is action docker image might not be following the global dockerignore and has unnecessary source code. Can be seen here: https://github.com/asyncapi/cli/actions/runs/17598055693/job/49994587866?pr=1844
- We don't require puppeteer for github-action I believe.
- What are the sizes of docker images now compared to before?
- What about the time taken?
| COPY --from=build /libraries /libraries | ||
|
|
||
| RUN cd /libraries && npm install --production --ignore-scripts | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't we need to install node_modules again?
| commands: | ||
| lint: | ||
| run: npm run lint | ||
| exclude: 'package-lock.json' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this needed?
|
Also there is conflict with package-lock. Tests should ideally pass upon pushing new commit. |



Description
Added dockerignore and copying only necessary files in Dockerfile
Related issue(s)
Fixes #1798
Inspired from #1800 (comment)