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

Settings rework #249

Closed
wants to merge 21 commits into from
Closed

Settings rework #249

wants to merge 21 commits into from

Conversation

dgdekoning
Copy link
Contributor

Some initial work on issue #106.

I've added variable typing to the settings classes and implemented functions from the commontasks.py file in the relevant settings classes.

While not quite the same as how sublime handles the settings files (no comments explaining the settings inside the file, etc.) I think this implementation does allow for ease of adding settings later on.

I anyone has suggestions for improvement please add them :)

@coveralls
Copy link

coveralls commented Jul 3, 2019

Coverage Status

Coverage increased (+3.4%) to 47.32% when pulling 6a68431 on dgdekoning:settings-rework into 9eaed7a on LCA-ActivityBrowser:master.

@dgdekoning
Copy link
Contributor Author

dgdekoning commented Jul 26, 2019

Alright, figured out why those two tests failed: The settings as initialized by the test_settings script are not using the pytest_project project but are instead using the 'empty' default project, meaning the valid_biospheres method specifically fails to find any databases.
Going to leave the alterations like this just to have that edge-case handled.

@dgdekoning dgdekoning marked this pull request as ready for review July 31, 2019 14:28
@dgdekoning dgdekoning self-assigned this Aug 5, 2019
@dgdekoning dgdekoning mentioned this pull request Oct 21, 2019
@dgdekoning
Copy link
Contributor Author

Closing this as another pull-request has already implemented it.

@dgdekoning dgdekoning closed this Dec 2, 2020
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