You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
My team was supposed to a code review of another project but we couldn't find the repository of the project. So I will do a code review of this project.
You have come a long way since I have my last review, I looked through the code and here is my review.
I can see that you have moved a lot of things to docker. Which is an excellent tool when it comes to scaling and deploying a distributed application. I noticed in a lot of DockerFile's the RUN command is on a higher layer as compared to the EXPOSE and EXPORT commands. Functionally doing this is ok, but when you are debugging the docker image, you want the RUN command to be in the higher layer as you can use the lower layers as the Base image. I would also recommend you to break down the RUN into smaller and more discrete commands, that way you will have more layers that would be reused when you update the Dockerfile.
A few python files have not been properly formatted, they have links, random strings, etc. If these are redundant, you could delete them or add them to the GIT LFS.
Most of the directories in the repositories house separate projects. It would make more sense to have separate repositories for each project. It would be much easier to maintain and scale the code base.
I observe a lack of build files and I can guess that you are building this project manually. I would recommend you to try certain build frameworks like Make or TUP.
The text was updated successfully, but these errors were encountered:
Hey! Thanks for the great suggestions! I was responsible for the Dockerfile and that suggestion would help us a lot. We were stuck just waiting for Docker to build the entire image again if we change anything in the lower levels.
We do have to work on our code organisation and formatting. We didnt really care about it when it was being developed but now that we're in the final stages, we will try to clean it up. 😀
And we did end up making our own version of a buildsystem - a buildscript rather. Definitely helped the setup process.
You have come a long way since I have my last review, I looked through the code and here is my review.
I can see that you have moved a lot of things to docker. Which is an excellent tool when it comes to scaling and deploying a distributed application. I noticed in a lot of DockerFile's the
RUN
command is on a higher layer as compared to theEXPOSE
andEXPORT
commands. Functionally doing this is ok, but when you are debugging the docker image, you want the RUN command to be in the higher layer as you can use the lower layers as the Base image. I would also recommend you to break down theRUN
into smaller and more discrete commands, that way you will have more layers that would be reused when you update the Dockerfile.A few python files have not been properly formatted, they have links, random strings, etc. If these are redundant, you could delete them or add them to the GIT LFS.
Most of the directories in the repositories house separate projects. It would make more sense to have separate repositories for each project. It would be much easier to maintain and scale the code base.
I observe a lack of build files and I can guess that you are building this project manually. I would recommend you to try certain build frameworks like Make or TUP.
The text was updated successfully, but these errors were encountered: