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

read profile from config.yaml and multiple file generating #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hitsmaxft
Copy link

@hitsmaxft hitsmaxft commented Jan 4, 2025

Pull Request Description

Title: Add CLI Argument for Config File Path, Verbose Logging, and Project Configuration

Description:
This pull request introduces several enhancements and configurations to the project:

  1. Command-Line Argument for Config File Path:

    • The script now accepts a command-line argument to specify the path to the configuration YAML file. If no argument is provided, it defaults to ./config.yaml.
    • This change allows for more flexibility in specifying different configuration files without modifying the script.
  2. Verbose Logging:

    • Added a -v or --verbose command-line argument to enable verbose logging.
    • When the verbose flag is set, the logging level is set to DEBUG, providing more detailed logs for debugging purposes.
  3. Project Configuration with pyproject.toml:

    • Created a pyproject.toml file to specify the project requirements, including the Python version and dependencies.
    • The project requires Python version >=3.10 and includes pyyaml as a dependency.

tests

the V3.xml is generated by new implemented python script, 100% equal to origin one

@hitsmaxft hitsmaxft changed the title support config.yaml and multiple file generating read profile from config.yaml and multiple file generating Jan 4, 2025
@hitsmaxft hitsmaxft force-pushed the master branch 7 times, most recently from 73b48d8 to b950710 Compare January 4, 2025 07:04
@BalzGuenat
Copy link
Owner

Hey, thanks for the PR! I am hesitant to merge it, though.

From a software engineering standpoint, these changes make sense and improve things, no doubt. But I feel they also bring a certain weight and lower the accessibility of this project.

Currently, I believe everyone who can manage to install Python will be able to download main.py, tweak some params and run the script to get their threads. I think this low barrier is a huge benefit.

With the changes you suggest, this barrier is raised. Someone wanting to generate their own threads will now additionally have to install a dependency and download and rename/copy another file (though the rename/copy could maybe be avoided). That is, if they don't go the proper and even more complicated route of cloning the repo and using Poetry, etc.

I know this may sound like childsplay to a semi-seasoned Python dev but I fear that to many in the "target demographic" of this project - the maker community - this might reduce its attractiveness significantly.

I hope I got my point across and would love to hear what your thoughts on my concerns are.

@hitsmaxft
Copy link
Author

hitsmaxft commented Jan 9, 2025

Your suggestion is very reasonable, I can replace the YAML file parser with the built-in JSON parser or other simpler format ,reducing the cost of non-Python writers, and keeping the original User's experience

I wanted a way to generate custom threads without a fork, so I provided a way to read the configuration file via parameters. Reading the default configuration file and generating the configuration by executing ”python main.py” should also be supported.

@hitsmaxft
Copy link
Author

hitsmaxft commented Jan 17, 2025

@BalzGuenat this pr is ready for reviewing
I've changed config format to JSON, and remove poetry files, since not library is required
profile for v3 generates same file content as current master

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.

2 participants