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

Split tables.py into several files #139

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

Conversation

encukou
Copy link
Contributor

@encukou encukou commented Dec 29, 2014

This splits off the Conquest tables into their own file.
Sanky's planned maps can also go into their own file.

Also, remove the test alphabetical order
@thechief389
Copy link
Contributor

This pull requests violates part of PEP 423 (the db package has another package inside it)

@magical
Copy link
Member

magical commented Aug 20, 2018

@thechief389

  1. PEP 423 is a guideline, not a rule
  2. The PEP status is Deferred, not Accepted
  3. pokedex has never been a model of good code organization imo
  4. this PR is over three years old

@encukou
Copy link
Contributor Author

encukou commented Aug 20, 2018

I also don't think PEP 423 is relevant. It is very far from being a collection of accepted best practices.
Also, I believe both reasons it gives for "Flat is better than nested" don't really apply in this case.

That said, I don't think this PR is useful. It has bit-rotted, and in case a a new category of tables needs to be added, recreating it might actually be easier than rebasing.

Feel free to re-open if you disagree.

@encukou encukou closed this Aug 20, 2018
@magical
Copy link
Member

magical commented Aug 20, 2018

@encukou I think it's still worth doing. Would you mind opening an issue?

@encukou
Copy link
Contributor Author

encukou commented Aug 21, 2018

Ah, OK. Then it's probably better as a PR, with a code example.

@encukou encukou reopened this Aug 21, 2018
@thechief389
Copy link
Contributor

This PR is three years old because nobody is here to work on this project. The only way to solve this problem is if there were more people working on it.

@magical
Copy link
Member

magical commented Aug 21, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants