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

[WIP - DO NOT USE] Pythonic and simpler refactor of wemod-launcher #138

Draft
wants to merge 77 commits into
base: main
Choose a base branch
from

Conversation

shymega
Copy link
Contributor

@shymega shymega commented Nov 17, 2024

Hi all!

As mentioned briefly in #100, and discussed over Discord with Marvin, this PR
is a overhaul I'm working on of wemod-launcher.

Testing is welcome once I get to a stage where it can be tested.

However, right now - it's a heavy WIP, and should not be used.

I have marked it as a draft PR.

It's worth noting that Nix is able to build this package on this PR natively,
so that is very cool.

Contributions welcome to my fork as PRs, which will then be pushed to this PR.

(Note: I will be rebasing and force-pushing. Let me know in advance before you
push to this branch if you're a maintainer, as your changes may be lost.)

Thank you!

shymega added 30 commits July 13, 2024 14:52
- Set +x on prepare.py (prev. setup.py).
- Set +x on wemod (symlinked to wemod.py as well).
- Rename setup.py -> prepare.py - otherwise conflicts
- Pythonize modules.

Signed-off-by: Dom Rodriguez <[email protected]>
This commit updates the names of imports from previously renamed
modules.

Signed-off-by: Dom Rodriguez <[email protected]>
Signed-off-by: Dom Rodriguez <[email protected]>
Temporary commit. To be reworded. TODO! FIXME!

Signed-off-by: Dom Rodriguez <[email protected]>
Signed-off-by: Dom Rodriguez <[email protected]>
Signed-off-by: Dom Rodriguez <[email protected]>
TOML is easier to read, and I hated dealing with YAML. PyYAML bad.

Signed-off-by: Dom Rodriguez <[email protected]>
Signed-off-by: Dom Rodriguez <[email protected]>
Signed-off-by: Dom Rodriguez <[email protected]>
Signed-off-by: Dom Rodriguez <[email protected]>
Signed-off-by: Dom Rodriguez <[email protected]>
@marvin1099
Copy link
Collaborator

I moved the nix files into ./tools/nix/
i have changed the paths to the nix files to reflect this,
but I'm not using nix so I'm not sure if i did this correctly.
If you have the time se if you can make it work.
I would be great if the nix files could be in a subfolder.

I also pushed the newest update and the license change to the refactor.
Hopefully you can also work with this.

Oh yes there are some helper scripts for running the wemod launcher script in ./script/ now.
That's probably not that relevant for you, but cant hurt to tell you.

Replaced poetry with PDM in pyinstaller.py
Added build helper script
Added PDM fail check in helper scripts
This new version uses the following syntax.
# First you import as usual 
from consts import Constants

# Then you can use the name of the setting you want like this
print(Constants.ConfigDir)

Very simple and intuitive
Some Of the constants are not jet added, 
right now there are only constants in here that can be defined without any other module.
So, some of the important constants still have to be added
The change of the config manager makes it now use:
from configuration import ConfigManager as Conf
Conf.LogFolder = "/test/path/"
print(Conf.LogFolder)

As this just saves the provided key something like this would work:
Conf.VeryLongRandomKeyHere = "mysetting"

To be implemented is a custom logger
In this file we still need saving and loading with the tomllib
I refactored const and configuration to work with instances,
its just a bit simpler in the const and configuration files.
A custom logger is now also added. 

Here is a example:
from consts import Constants
from configuration import ConfigManager
from logger import Logger
CT = Constants() # Get a const instance
CM = ConfigManager() # Get a conf instance
LG = Logger() # Get a logger instance
print(CT.config_path) # print config path
print(CM.log_name) # print active log file
LG.info("Test message", "test") # Push a log message from module "test"
CE = ConfigManager(env_vars=True,cmd=True) # load environment variables (priority) too   
print(CE.XDG_CACHE_HOME) # print a env var
@shymega
Copy link
Contributor Author

shymega commented Dec 7, 2024

I think I would rather you push your commits to your own branch, based off my PR, and create PRs from your branch's HEAD, to my PR. Otherwise with my force-pushing, you will lose your changes.

As for the Nix changes, this isn't how Nix works. It breaks everything. I propose you checkout a new branch locally on your machine, and once you've done that, create a PR with the base branch set to this PR and the HEAD set to your new branch. Then we can merge things into this master branch as we go.

@marvin1099
Copy link
Collaborator

I think I would rather you push your commits to your own branch, based off my PR, and create PRs from your branch's HEAD, to my PR. Otherwise with my force-pushing, you will lose your changes.

So make changes on my copy and then merge them to this branch, shure.

As for the Nix changes, this isn't how Nix works. It breaks everything. I propose you checkout a new branch locally on your machine, and once you've done that, create a PR with the base branch set to this PR and the HEAD set to your new branch. Then we can merge things into this master branch as we go.

I expected something to break I'm not a nix expert, I was kinda hopping is was not that far off from working, and that you could make it work,
do all nix files have to be in the project root then?

Merging into master, well that is still going to take a bit i think.

Well as for the fix files can you do something to clean the root up a bit, would be great.

As shymega pointed out it breaks stuff on nix.
I would still like the files to be in a subfolder but im not a nix guy.
So shymega hopfully can fugure something out.
But for now i will revert that.
@marvin1099
Copy link
Collaborator

For now i reverted the nix file move, if you figure something out it would be nice if you can make it happen.

@Marakai
Copy link

Marakai commented Jan 13, 2025

Hi! I just forked the repo as I am interested in working on some approaches to automate and improve installation and use. Things like using Poetry and/or Docker.

Should I do that off main or this WIP branch (despite it not being fully usable right now)?

@marvin1099
Copy link
Collaborator

Hi! I just forked the repo as I am interested in working on some approaches to automate and improve installation and use. Things like using Poetry and/or Docker.

Should I do that off main or this WIP branch (despite it not being fully usable right now)?

Hi @Marakai,
you can work on this refactor branch (WIP branch) or the Dev branch.
Keep in mind that we actually use pdm not poetry in the this branch, please use that instead.
Also keep in mind that the Dev branch is the working and currently used branch by the comity.
Also the dev / main branch will be deprecated in the future.

This branch is what the future of wemod will be but it is not only WIP I'm also pretty shure it won't run yet, so you may end up trying to figure that out how to run it.

But if you want to help for the future then help with the refactor otherwise you can try to make the old and active version in Dev and main as good as they can be, feel free to work on what you like where.

@shymega
Copy link
Contributor Author

shymega commented Jan 13, 2025 via email

@shymega
Copy link
Contributor Author

shymega commented Jan 13, 2025

I think I would rather you push your commits to your own branch, based off my PR, and create PRs from your branch's HEAD, to my PR. Otherwise with my force-pushing, you will lose your changes.

So make changes on my copy and then merge them to this branch, shure.

As for the Nix changes, this isn't how Nix works. It breaks everything. I propose you checkout a new branch locally on your machine, and once you've done that, create a PR with the base branch set to this PR and the HEAD set to your new branch. Then we can merge things into this master branch as we go.

I expected something to break I'm not a nix expert, I was kinda hopping is was not that far off from working, and that you could make it work, do all nix files have to be in the project root then?

Merging into master, well that is still going to take a bit i think.

Well as for the fix files can you do something to clean the root up a bit, would be great.

I've been giving this some thought.

I think, if you can create a refactor-marvin branch on this repo, based off this branch, I will then push to this PR from my commits, and rebase your commits onto this as and when it's necessary.

Otherwise, we end up pushing over each other in a PR.

Do you mind if I revert some of your commits and remove them from the commit history, just until it's in a better state - the Nix ones, for example. There's also a lot of merge conflicts that weren't there when it was just me pushing - but I can't recall precisely.

@Marakai
Copy link

Marakai commented Jan 14, 2025

Hi! I will reply in the Help Wanted issue.

@marvin1099
Copy link
Collaborator

Do you mind if I revert some of your commits and remove them from the commit history, just until it's in a better state - the Nix ones, for example. There's also a lot of merge conflicts that weren't there when it was just me pushing - but I can't recall precisely.

Yep if needed you can revert some stuff, I thought I reverted far enough to make it work. I will make the refactor-marvin branch and add my changes there.

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.

3 participants