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

refactoring #3

Closed
wants to merge 1 commit into from
Closed

refactoring #3

wants to merge 1 commit into from

Conversation

garak
Copy link

@garak garak commented Apr 7, 2020

This is a first attempt to improve code base, trying to split god class Chess into smaller classes, with less responsibilities.
Some other improvements in code readability (mainly type hinting) is also included.

I have a few ideas for next steps (so for subsequent PRs)

@akondas
Copy link
Owner

akondas commented Apr 7, 2020

Hey @garak, thanks for contribution, actually this fork was created mainly for performance reasons, I don't know if the flowery code readability is better, but this library is versioned anyway so I'd love to see where it takes us 😉

@akondas
Copy link
Owner

akondas commented Apr 7, 2020

possibly, if you only want to, can we make a separate library out of it? (with separate independent namespace), for example: chessphp/engine or something like that

@garak
Copy link
Author

garak commented Apr 8, 2020

possibly, if you only want to, can we make a separate library out of it? (with separate independent namespace), for example: chessphp/engine or something like that

That's feasible of course, but what's the purpose?

@akondas
Copy link
Owner

akondas commented Apr 8, 2020

Clean and solid PHP chess engine for everyone? I leave the decision to you, we can either merge it or do a separate project, let me know.

@garak
Copy link
Author

garak commented Apr 8, 2020

I see, maybe we can drop "Ryans" from namespace? I'll integrate my PR if so

@garak
Copy link
Author

garak commented Apr 8, 2020

So, the choice now would be: how to name it? PHP folks prefer avoiding the use of "PHP" in library names, so "ChessPHP" is not suitable.
What about simply "Chess"?

@akondas
Copy link
Owner

akondas commented Apr 8, 2020

The name 'chess' is already taken.

@garak
Copy link
Author

garak commented Apr 8, 2020

PChess?

@akondas
Copy link
Owner

akondas commented Apr 8, 2020

and the repository name? chess?
For example: pchess/chess

@garak
Copy link
Author

garak commented Apr 8, 2020

and the repository name? chess?
For example: pchess/chess

I guess that we can leave that for a second step. For now, we can keep github under akondas/chess.php and using Pchess\Chess as namespace.
When we're ready, we can move to a new repo with consistent name.

If instead you prefer to move now, no problem for me. Create the new repo and let's go with pchess/chess. In such case, you can close this PR and I'll re-do it on such new repo

@akondas
Copy link
Owner

akondas commented Apr 8, 2020

Bad luck: The name 'pchess' is already taken.

@garak
Copy link
Author

garak commented Apr 8, 2020

On packagist is still available https://packagist.org/explore/pchess (I see a 404)
I guess that here on github you can be creative 😉

@akondas
Copy link
Owner

akondas commented Apr 8, 2020

php-chess or chessphp,

@garak
Copy link
Author

garak commented Apr 8, 2020

What about p-chess?

@akondas
Copy link
Owner

akondas commented Apr 8, 2020

https://github.com/p-chess/chess (check e-mail for invitation), thanks for your patience and cooperation

@garak
Copy link
Author

garak commented Apr 8, 2020

Here we are p-chess/chess#1

@garak garak closed this Apr 8, 2020
@garak garak deleted the refactoring branch April 8, 2020 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants