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

Refactor presets module #810

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

lavafroth
Copy link
Contributor

Description

This PR involves replacing a bunch of manual implementations for copying, removing, checking extensions and the like with use of built-in python modules. The JSON reading and parsing is also improved to consume less memory.

Type of change

  • Bugfix (Change which fixes an issue)
  • New feature (Change which adds new functionality)
  • Enhancement (Change which slightly improves existing code)
  • Breaking change (This change will introduce incompatibility with existing functionality)

Changelog

  • Uses the copyfile function from shutil to copy the GTK CSS file to its backup and vice versa.
  • Moves the __get_repo_presets inner function outside and allow it to mutate a dictionary passed to it.
  • Factors out the common functionality of loading presets into its own method.
  • Uses the suffix attribute for pathlib.Path to check for the file extension instead of instead of stringifying it.
  • Uses os.makedirs with exist_ok set to True to create directories if absent.
  • Reads JSON structure directly from file handles using json.load(file) instead of reading the whole file and then parsing.
  • Uses the in keyword to check if a key is in a dictionary instead of explicitly using the get method.

Testing

  • I have tested my changes and verified that they work as expected

How to test the changes

No information provided.

@tfuxu tfuxu self-requested a review August 18, 2023 19:09
@tfuxu tfuxu added this to the 0.8.0 milestone Aug 26, 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