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

added the possibility to load multiple INI files #20

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

Conversation

thorsten-klein
Copy link

@thorsten-klein thorsten-klein commented Mar 24, 2020

  • added the possibility to load multiple INI files from the folder /etc/commonapi-dbus.d or the folder provided by the environment variable COMMONAPI_DBUS_CONFIG (if it is a folder).

Priority is as follows:

  1. folder definded by COMMONAPI_DBUS_CONFIG
  2. file in the current folder
  3. file definded by COMMONAPI_DBUS_CONFIG
  4. folder /etc/commonapi-dbus.d
  5. file /etc/commonapi-dbus.ini
  • added the capability to use ; for comments

…tc/commonapi-dbus.d or the folder provided by the environment variable COMMONAPI_DBUS_CONFIG (if it is a folder). Priority is as follows: 1. folder definded by COMMONAPI_DBUS_CONFIG 2. file in the current folder 3. file definded by COMMONAPI_DBUS_CONFIG 4. folder /etc/commonapi-dbus.d 5. file /etc/commonapi-dbus.ini - added the capability to use ; for comments
@thorsten-klein thorsten-klein changed the title - added the possibility to load multiple INI files added the possibility to load multiple INI files Mar 24, 2020
@goncaloalmeida
Copy link
Contributor

@thorsten-klein please provide some examples/proof of these changes working.

Copy link
Contributor

@goncaloalmeida goncaloalmeida left a comment

Choose a reason for hiding this comment

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

already requested for examples..

@thorsten-klein
Copy link
Author

Are there already some tests existing for the current commonapi-dbus.ini? Then I can add some tests for my change analog to existing tests.

@goncaloalmeida
Copy link
Contributor

@thorsten-klein
Copy link
Author

But are there already some specific tests for the current ini config file loading?

@goncaloalmeida
Copy link
Contributor

@thorsten-klein I suppose not.
But we are not using this internally (multiple ini files) so we need proof that this is working :)

@thorsten-klein
Copy link
Author

Yes I fully understand. If it does not break your setup, then it could actually be merged 😎
If it is too risky for you, then we need your support in order to create a test for this.

It is very hard for me to create a test for this extended feature, if there is no test at all for the current (simple) commonapi-dbus.ini loading. This would mean I have to work out a test concept how to test these config files. So would be nice if you create a test for the simple commonapi-dbus.ini loading, so I can set up on top of your work.

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.

3 participants