search

yaqwsx / PcbDraw

Convert your KiCAD board into a nicely looking 2D drawing suitable for pinout diagrams
MIT License
1.16k stars 91 forks source link

Fix executable permissions for pcbdraw/*.py scripts #122

Closed set-soft closed 2 years ago

set-soft commented 2 years ago
yaqwsx commented 2 years ago

Does it really make sense to have ui.py executable? Couldn't the user just invoke python ui.py or python -m pcbdraw? What do we gain by making it executable?

set-soft commented 2 years ago

Does it really make sense to have ui.py executable? Couldn't the user just invoke python ui.py or python -m pcbdraw? What do we gain by making it executable?

Nothing special, as populate.py is executable, ui.py could be also executable. They can be executed so:why not marking them as executables? On the other hand plot.py can't be executed. Marking them executable help people to know they can be invoked, and cost nothing, is just an attribute.

set-soft commented 2 years ago

A couple of detail that may be could help:

  1. In the current repo plot.py and populate.py are executable. But only populate.py has an if __name__ == '__main__':. This patch removes the plot.py permissions and adds them to ui.py, which also has if __name__ == '__main__':
  2. I have another patch to allow running populate.py and ui.py in-place, without needing to install. Quite handy when you want to run a quick test, you just check out a point and run the script, no need to install. Note: I know about pip install -e, but it has its own drawbacks.
yaqwsx commented 2 years ago

The argument with having the main routine is sufficient for me. I agree that the plot.py definitely shouldn't be executable. Personally, I don't have a use case for executing them as scripts (as they have entry points in setup.py), but I guess it doesn't hurt.

set-soft commented 2 years ago

but I guess it doesn't hurt.

Exactly, small things that doesn't need big effort can help people for free. When I checked out 1.0.0 I got confused by the file permissions. Thanks

yaqwsx commented 2 years ago

On the other hand, pulling stuff that the maintainers aren't used to can also cause damage. I missed the permission on plot.py as this is something I don't do for python packages and there are no tests for testing files being executable.