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

Contribution to the projects Software Engineering: implementation of CI using Docker #15

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

AnHoff
Copy link

@AnHoff AnHoff commented Jul 2, 2023

Description

Me and my friend Eduardo Schuindt have made a contribution to CoCa-pytorch as a class project in our University. Our contribution implements CI in your project's pipeline, using Docker to build the application and deploy the library to PyPI.

Although our knowledge in your project's field is limited, we could contribute to your project's software engineering, enhancing its quality and performance. We used Sphinx to create an automated documentation, Poetry to build and publish the Python package to PyPI and Docker to create containers. Also, it was necessary to re-arrange some folder's structure to assure our features would work properly. Everything we did can be easily changed and enhanced as needed in the future.

There are still some enhancements to be done. Some adjusts may be needed to finish poetry publish (as it requires authentication of the publisher), implement tests (they should be working in our pipeline as soon as you create some tests in the tests folder) and, if you wish, there is already a Docker container to build a database.

Using Docker build processes simplifies the deployment procedures and creates consistency across different development environments, so we strongly recommend using it.

This pull request has all the necessary changes. Your main code file is now named main.py and is in the folder named src. There were also adjustments to readme, making it easy to follow the steps needed. The requirements were also corrected, as it used to have compatibility problems.

This PR has only 1 commit due to the professors requirements. The complete history may be found in this repository.

We would be grateful if you reviewed this PR and gave a feedback or approval.

All the best,
Ana Hoffmann and Eduardo Schuindt

Co-authored-by: edudsan <eduschusan@gmail.com>
@lucidrains
Copy link
Owner

lucidrains commented Jul 2, 2023

nice! A for effort! so on cursory examination, why is a mysql database involved? also, what do you think of pip-tools or rye? poetry critique

@AnHoff
Copy link
Author

AnHoff commented Jul 2, 2023

Poetry was our professor's recommendation and after looking at other tools for a while we decided it was the easiest to use. The mysql db was a feature required by our professor. Also, you don't use mysql in your project so it may not be needed now and can be deleted (or ignored). On the other hand, if you decide to create a db, it already has the necessary to run as a docker container. If you don't plan using it at all, I 100% recommend removing it.

I had no problems at all with poetry, so it's my personal choice. Also, I have worked with other projects that used poetry and none of them faced any issues. The other tools seems to be nice too but, as I said, it's a personal choice and I don't really have experience with them. If you think other one would be better, let me know about it!

@lucidrains
Copy link
Owner

@AnHoff sounds good Ana! let me circle back to this after the holidays and we can try to get it merged!

@AnHoff
Copy link
Author

AnHoff commented Jul 4, 2023

Soon I'll have more free time to work on the issues present on the code. If you wish to remove the database part just let me know. There should be no urge to merge this branch into main.

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.

None yet

2 participants