MrPigss / BetterJSONStorage

Better JSONStorage for tinyDB
https://pypi.org/project/BetterJSONStorage/
MIT License
33 stars 4 forks source link

duplicated code #1

Closed GavinHuttley closed 2 years ago

GavinHuttley commented 2 years ago

First, providing a more faster / more compact backend to TinyDB is a great idea. Nice job!

This issue is posted because you have duplicated your code:

Both source files are identical. Your setup points to the latter so I suggest you delete the top-level.

The reason I point this out is I'd like to adopt this code in a project, but there is a feature I'd like to request (possibly contribute) but the code duplication is a barrier.

I have some other project organisation suggestions, but I'll leave it at one issue for now.

MrPigss commented 2 years ago

Yes you are right. I'll fix it with the next commit. This was my first real project so I was still learning about project organization. Any suggestions would be very appreciated.

GavinHuttley commented 2 years ago

Well congrats on a very nice first project!

I think the most important thing (for your sanity) is to get continuous integration going. Here's an example GitHub actions workflow from one of my main repos. It's checking against all major OS'es and across multiple versions of Python. That way any breaking changes you make will be caught. Please note I'm not proposing this as the best example out there, but it's one I know...

In this case, I notice (for instance) that setup.py indicates support for python 3.6+, but the code uses :=, which is 3.8. You should test against all versions you intend to support. Don't be afraid to drop support for older versions.

In terms of organisation, I blogged on this for undergrad science students. Much of the post is not relevant to you, but the sample project structure for a software methods project is.

Some other points:

MrPigss commented 2 years ago

Thanks for the tips!

I'll be looking for a good continuous integration solution when I have a bit more time on my hands.

For now I think the main issue should be solved so I'll close it. Thanks again for your helpfull points.