bcollazo / catanatron

Settlers of Catan Bot Simulator and Strong AI Player
https://catanatron.com
GNU General Public License v3.0
278 stars 64 forks source link

refactor server files #283

Closed HassanJbara closed 2 months ago

HassanJbara commented 2 months ago

I wanted to do this before adding more endpoints and functionality to the server, since I think this structure is more orderly and easier to work with.

netlify[bot] commented 2 months ago

Deploy request for catanatron-staging pending review.

Visit the deploys page to approve it

Name Link
Latest commit 5639c42a9a64711e534cff6fbe0d0a44bb5201ef
HassanJbara commented 2 months ago

Can you maybe add a section in the readme about the automated tests? For example, I use black-lint myself, so I'm not sure what kind of rules the linter in this pipeline is using and why it keeps failing 😅.

zarns commented 2 months ago

Note: the readthedocs build failure should be resolved in this change: https://github.com/bcollazo/catanatron/pull/281#issuecomment-2241748735

bcollazo commented 2 months ago

Hey @HassanJbara thanks for the PR! That being said, I'm not aligned with this refactoring, so I will kindly close. The api module is already the "view" module (I'm a fan of avoiding 1-line functions that call other functions), and I prefer the original naming.

I see this would also block the Pickle work... so sorry about that. In general, I'd recommend having contributions keep the original architecture / naming structure of the project for now to increase the chances of me accepting. Thanks!