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

Redundant variables #36

Closed
TomCasavant opened this issue May 15, 2024 · 8 comments · Fixed by #38
Closed

Redundant variables #36

TomCasavant opened this issue May 15, 2024 · 8 comments · Fixed by #38
Labels

Comments

@TomCasavant
Copy link
Owner

There's a few variables that should be constants instead of being redefined in every function e.g.

script_dir = os.path.dirname(os.path.realpath(__file__))

@lukemurphy147
Copy link
Contributor

lukemurphy147 commented May 31, 2024

Hi Tom, I'd like to work on this issue. Is there anything I'd need to download or install in order to test my changes, besides the repo itself?

@TomCasavant
Copy link
Owner Author

Hi Tom, I'd like to work on this issue. Is there anything I'd need to download or install in order to test my changes, besides the repo itself?

When you clone the repo you'll want to do pip install -r requirements.txt and then set up your config file, unfortunately for legal reasons I can't provide you with a rom file so you'll have to source one yourself.

If you want to test the mastodon side you'll need a bot account on a mastodon server (if your server doesn't support more than 4 poll options you'll have to work off of @phi1997's pull request which hasn't been merged in yet #33. Fortunately for this issue you probably won't need a mastodon account, just uncomment

# bot.test()
and comment out the following line

Worst case, if you just want to make the updates and put them in a pull request I'll go ahead and test them for you

@lukemurphy147
Copy link
Contributor

Is this the right place to enter that pip command? (I redacted my computer name)

Untitled

@TomCasavant
Copy link
Owner Author

Is this the right place to enter that pip command? (I redacted my computer name)

Yeah that's where you'd run it from (https://note.nkmk.me/en/python-pip-install-requirements/),
Are you on Windows? I haven't tried it yet, so I'm not 100% certain all the packages work there.

Also, if you're new to this I'd also recommend setting up a virtual environment (https://docs.python.org/3/library/venv.html) and you'll want to make sure you're running a python version >3.9

If you're not familiar a virtual environment is pretty easy to setup if you have python installed already:
python -m pip install virtualenv
python -m virtualenv .venv # Creates a virtual environment in the .venv directory
.\.venv\Scripts\activate # Starts the virtual environment (in linux this would be source .venv/bin/activate)

And then when you're in the virtual environment install all the packages with the pip install -r requirements.txt command

@lukemurphy147
Copy link
Contributor

lukemurphy147 commented Jun 12, 2024

Okay, I finally got around to working on this. So far I've made script_dir constant by moving it above the classes, but I'm not sure I can do the same with screenshot_dir and gif_dir because they depend on function parameters. Should I go ahead with the pull request, or am I missing something? Also, which branch should I be working on?

@TomCasavant
Copy link
Owner Author

Okay, I finally got around to working on this. So far I've made script_dir constant by moving it above the classes, but I'm not sure I can do the same with screenshot_dir and gif_dir because they depend on function parameters. Should I go ahead with the pull request, or am I missing something? Also, which branch should I be working on?

Yeah I might change how screenshot_dir and gif_dir work later on but that's fine for now could you move save_dir as well I'm pretty sure that's constant throughout the project? And then yeah that should be good for a PR

@lukemurphy147
Copy link
Contributor

I think you meant save_loc, so I've moved that, ids_loc, and gif_dir (turns out I could move it in bot.py), but I still need to know which branch to put the PR on

@TomCasavant
Copy link
Owner Author

Just the main branch is fine

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

Successfully merging a pull request may close this issue.

2 participants