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 of repo structure and adding some best practices #2

Merged
merged 10 commits into from
Oct 11, 2023

Conversation

chaeynz
Copy link
Contributor

@chaeynz chaeynz commented Oct 11, 2023

@lookthis664
Request to refactor the repo structure
I changed some filenames, added some best practices.

and introduced a build script for compilation to an executable file

Source code should generally be in src/ so I moved the files from modules/ into there
Also I moved the webhook.txt to config/ instead of config/src
src, again, is where your source code is located. webhook is not sourcecode, but instead a config file.

The main.py file was renamed to colibris.py.
Also because of this, when the file is compiled it will create an executable named "colibris"

You should check the build.ps1 Powershell file, since I don't have a windows system handy right now, but that's like 1 change, if I made errors.

You forgot specifying encodings when writing to files.
According to Best Practices you should specify encodings every time.

You had pycache in your repo. Those are temporary files that should not be included in the repo.
I added a .gitignore to address this.
Also I untracked the bin/ and build/ folders in the .gitignore.
Those are the folders used by pyinstaller as specified in the build scripts.

Finally, you had a bat script that was responsible for the config.

  1. Python is supposed to be cross-platform, so I would generally advise using multiple scripts, if they are necessary
  2. The bat file is not necessary, since this can be done with a requirements.txt. I introduced this file in f1cbcd1.

PS: I changed a few things in the README, so usage instructions should be up to date

created requirements.txt instead, following Python Best-Practices
changed usage instructions
temporary files generated during build process by pyinstaller
changed all references to this location accordingly
also specified encoding as UTF-8 everywhere, following Python Best-Practices
Copy link
Owner

@lookthis664 lookthis664 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your support I'm going to see this again tonight to see what I could change with regard to my expectations

@lookthis664 lookthis664 merged commit 0575619 into lookthis664:main Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants