-
Notifications
You must be signed in to change notification settings - Fork 123
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
Modular module system #380
base: master
Are you sure you want to change the base?
Conversation
modular system for importing modules
Modular import working
@mrkipling @N3MIS15 You should check this out. Small change that makes a big difference in modularity for new modules. Would like a second opinion before merging 😄 |
Might be wise to have a sample and add the inis to git ignore? |
@@ -33,3 +33,11 @@ ehthumbs.db | |||
Thumbs.db | |||
.directory | |||
*~ | |||
.idea/.name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these additions to .gitignore needed? I can't see what they're for.
Wouldn't that mean that the modules don't load out of the box, though? The way I see it this isn't really so that users can edit the ini files. |
I haven't tested this yet, but generally it gets my +1! |
I would be happy to review, but I won't have an opportunity until the weekend. @gugahoi ping me on the weekend so I remember. |
I was under the impression that the users would edit the inis, if that isnt the case there would no reason to add it to git ingore. |
It's entirely possible that I'm just being dense, it's early and I'm still on coffee 1 :) If users change settings by editing the ini files, does this mean that you can no longer change settings from within the interface? Because that wouldn't be so great. Perhaps we could get some clarification on what exactly is proposed here? Modular modules are good; extra complexity for the user is bad. |
I will clarify some stuff for you guys: The ini files are simply the default settings for the modules, user settings still reside in the db. All this does is split the long ass array of default settings we used to have into their dedicated files. Makes it easier to manage individual modules. That stuff added to gitignore it not necessary, his own IDE added that in. It's great to hear your opinions tho 👍 |
Fantastic, this is my original assumption. I'm all for that, sounds like a great idea. |
@@ -1040,6 +340,7 @@ def extra_settings_dialog(dialog_type, updated=False): | |||
|
|||
if dialog_type == 'search_settings': | |||
settings = copy.copy(SEARCH_SETTINGS) | |||
print settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this shouldn't be here..
without testing, In theory it looks sound. |
Sorry bout that, thought i removed all of them. Will get that out asap
|
Checked .ini files for other python code execution, which is possible, however the .ini files are there for default config, which shouldn't contain any other information, can rename them all to something different if needed.
Removed the print statements ( all of them that i put in ) the .idea folder contains a lot of information from my IDE, didn't want that synced in so i added it to my .gitignore, the gitignore additions can be ignored. I have been looking into a better way to make this more modular, however there are some files that look for the files in the location they are right now, to get this done i have to check what files they are and modify them as well. The only downside i noticed is the sorting of the list in the GUI, currently this is not in the same sorting as it used to be as it is now generated. |
Could you adjust the PR to remove these additions please? FYI you can add personal files that you want excluded to |
Created a system that keeps the module config with the modules, loads the modules that have an ini file with them.
ini file can be blank as well when the module doesn't require any configuration.