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

Integrate new arguments data structure #3167

Merged
merged 9 commits into from
Feb 19, 2025

Conversation

svartkanin
Copy link
Collaborator

This is the integration of the new ArchConfig data structure in favor of the archinstall.arguments structure as that one is merely a dict[str, Any] which doesn't allow for any type checking and safety.

Unfortunately, this PR grow beyond what I originally intended to change here but by introducing the args.py as a dependency (which pulls in most data models) there were loads of circular dependencies to be dealt with as the code base isn't well divided hierarchically at the moment so I was forced to move things around to get the interpreter happy. Most notably I moved a lot of model data classes into the archinstall/lib/models folder. This aligns with existing models in there as well.

Changes

  • Replaced all usages of the archinstall.arguments dict type and replaced it with the respective config or args types
  • Version is no longer explicitly defined in archinstall.__init__ but will be read in the args.py file directly from the pyproject.toml @Torxed let me know if that causes issues on release
  • Moved data types from disk/* to models which resolved the circular dependencies
  • Added tests for the output testing of the new arguments structures

As I have only moved the relevant data types into the models/ module there's still some misalignment in the code base, I'm planning to raise a follow up PR to cleanup the rest of the types as well and move them into the models if they fit to avoid any major future dependency nightmares.

@svartkanin svartkanin requested a review from Torxed as a code owner February 9, 2025 10:40
Copy link
Member

@Torxed Torxed left a comment

Choose a reason for hiding this comment

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

It's a big one for sure.
But it looks good, lets merge it.

@svartkanin svartkanin merged commit b57f7f9 into archlinux:master Feb 19, 2025
8 checks passed
@svartkanin svartkanin deleted the integrate-args branch February 19, 2025 20:36
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