kbr / fritzconnection

Python-Tool to communicate with the AVM Fritz!Box by the TR-064 protocol and the AHA-HTTP-Interface
MIT License
304 stars 59 forks source link

Add docker file for quick VS code development #103

Closed chemelli74 closed 2 years ago

chemelli74 commented 2 years ago

Allow a easy "Clone repository in container volume" from VS code.

Simone

chemelli74 commented 2 years ago

@kbr do you think this PR can be merged ? Nothing changes for users that don't rely on VS code.

Simone

kbr commented 2 years ago

@chemelli74: I wish you have started a discussion first before putting much work in a lot of pull requests. Regarding development environments I would like to keep the project as clean as possible, so in the best case the repository contains just code.

By peeking in .gitignore one can see, that I'm on a mac, used jupyter and also VSCode (but I'm back to another editor). I try to keep all this out from the repo, because other projects and programmers use other tools.

The same for a docker configuration. It's ok to set up a docker file for a larger project that may include fritzconnection, but for development of a Python library virtual environments are the proper tool. It also doesn't matter whether you work with Python 3.8 or another version, as long as tox handles the tests for different versions (see tox.ini).

chemelli74 commented 2 years ago

IS really a pity as changes are limited to 2 files in a single folder. And makes life a lot easier for those that use the recommended HA development environment (which is the dev container).

With a single click you get everything setup and you are ready to debug or coding.

But as it's your project, if you feel is not needed go ahead and close the PR.

Simone

kbr commented 2 years ago

Basically you are right, but these two files are HA project specific. Consider a change there with a fritzconnection update to a new version, even without any other code changes. That would be very confusing for all other users. And which project will be next to include docker files?

chemelli74 commented 2 years ago

Basically you are right, but these two files are HA project specific. Consider a change there with a fritzconnection update to a new version, even without any other code changes. That would be very confusing for all other users. And which project will be next to include docker files?

You should not include those file in the release, neither release a new version for that. Those 2 files just need to be on the repository site, nothing else.

Simone

kbr commented 2 years ago

Files in the repository are part of the project and part of every release. The mentioned files do not belong to this project. They are configuration files related to another project. That's not a clean separation and should get solved otherwise.

chemelli74 commented 2 years ago

I don't want to upset you in any way, but I think this is not the case:

https://github.com/kbr/fritzconnection/blob/1b47528f7ac4ad7707dadb03ea2f1efcd059e106/setup.py#L24

setup takes the folder fritzconnection, excluding tests. So nothing else is part of the release. Am I wrong ?

Also, docker file is not coming in any way from HomeAssistant. How I created them ? Just exec "Clone repository in Container Volume..." from within VS code: both files will be created. I just uncommented the run pip requirements, and that's it ;-)

Simone

chemelli74 commented 2 years ago

If you have time I wish to discuss about that with you. You find me on Discord: 'chemelli74#2180'

Simone

chemelli74 commented 2 years ago

Can you please be so kind and consider merging this one ?

Simone

chemelli74 commented 2 years ago

Can we speak about this one ?

I would like to start working on logging but is not at all simple without.

Thx,

Simone

chemelli74 commented 2 years ago

Hi @kbr, can you please consider merging this PR so I can start working on logging ? ;-) Thx in advance for your time and help.

Simone

chemelli74 commented 2 years ago

No interest on implementing, closing

A real pity as this will speed up development for VS code and Docker users. ref: https://github.com/kbr/fritzconnection/pull/103#issuecomment-877097042

Simone