-
Notifications
You must be signed in to change notification settings - Fork 219
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
Containerize #505
base: development
Are you sure you want to change the base?
Containerize #505
Conversation
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
Deploy preview for donut-frontend failed. Built with commit f67e3cb https://app.netlify.com/sites/donut-frontend/deploys/5f07415aaa833e000876b193 |
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
|
||
WORKDIR /server/social-platform-donut-frontend | ||
|
||
RUN npm install && \ |
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.
be verbose with npm version
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.
@vaibhavdaren
Are you pointing to the tag --loglevel
?
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.
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.
@vaibhavdaren Thanks for the link, I have changed node from LTS to 14 so this will be always using a specific version of npm which is v6 (Major).
However, the link mentions about using a higher version of npm outside the lower node versions. I guess now we have got v14 (three years later) with stringent npm v6 within the container. So. we no need to install again npm as per the link. Does this sound right?
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
Please attach readme link here |
@vaibhavdaren Please let me know any other tests we can write or have to for docker files (PS: I am not sure of any other than using a linter) other than testing locally or within the Actions pipeline by building the image. |
I means that we need to test locally before merging the pr. Thats it. |
@vaibhavdaren Please don't merge, I need to fix the pipeline a bit 😅 |
Please ask @jaskiratsingh2000 to run locally and merge. |
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
@vaibhavdaren @jaskiratsingh2000 Can you please review this by locally running this feature, I have done it from my side anyways. |
I would be able to do probably by wednesday. @devesh-verma will respond soon |
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
Hi, @kmehant After struggling so much with my docker desktop and testing it threw an error for me. I have 2 major concerns:
Thanks |
@jaskiratsingh2000 FYI -
Answering your concerns: Any more concerns please let me know. |
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.
@kmehant @jaskiratsingh2000 Please re-iterate on docker images.
[1] Proper documentation
[2] Test cases are running
[3] Recreate-DB or loading sample database works
cc @Rupeshiya @AuraOfDivinity
After docker-compose what steps are missing? ==> before a new contributor can start the development?
Signed-off-by: K mehant 411843@student.nitandhra.ac.in
Todo list:
DOCKER_USERNAME
andDOCKER_PASSWORD
to this repository