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

JSONConfiguration in Poco::Util::Application #7

Closed
wants to merge 2 commits into from

Conversation

naquin
Copy link

@naquin naquin commented Nov 13, 2012

I have added JSONConfiguration to Poco::Util::Application::loadConfiguration. This seems to be a reasonable extension since JSONConfiguration was already implemented.

I also added a corresponding POCO_UTIL_NO_JSONCONFIGURATION macro along the lines of POCO_UTIL_NO_XMLCONFIGURATION and POCO_UTIL_NO_INIFILECONFIGURATION.

Please let me know if this is a good idea or not. Thanks!

@obiltschnig
Copy link
Member

sounds good to me

-guenter

Günter Obiltschnig
Applied Informatics Software Engineering GmbH
A-9182 Maria Elend | Maria Elend 96/4 | www.appinf.com
P: +43 4253 32596 M: +43 676 5166737 F: +43 4253 32096

Company Registration: FN 276491 f | Landesgericht Klagenfurt
Managing Director: DI Günter Obiltschnig

On 13.11.2012, at 22:46, syvex wrote:

I have added JSONConfiguration to Poco::Util::Application::loadConfiguration. This seems to be a reasonable extension since JSONConfiguration was already implemented.

I also added a corresponding POCO_UTIL_NO_JSONCONFIGURATION macro along the lines of POCO_UTIL_NO_XMLCONFIGURATION and POCO_UTIL_NO_INIFILECONFIGURATION.

Please let me know if this is a good idea or not. Thanks!

You can merge this Pull Request by running:

git pull https://github.com/syvex/poco JSONConfiguration
Or view, comment on, or merge it at:

#7

Commit Summary

• Add JSONConfiguration to Poco::Util::Application::loadConfiguration.
• Update description for POCO_UTIL_NO_JSONCONFIGURATION
File Changes

• M Foundation/include/Poco/Config.h (6)
• M Util/src/Application.cpp (12)
• M configure (2)
Patch Links

https://github.com/pocoproject/poco/pull/7.patch
https://github.com/pocoproject/poco/pull/7.diff

Reply to this email directly or view it on GitHub.

@aleks-f
Copy link
Member

aleks-f commented Nov 13, 2012

it is a good idea;however, to build Util without JSON, the JSONConfiguration itself should be #idef'd as well. can you please do that as well?

@aleks-f
Copy link
Member

aleks-f commented Nov 13, 2012

additionally, please fork code off the develop branch (not release)

see this model, that's what we use:

http://nvie.com/posts/a-successful-git-branching-model/

this page is also helpful to stay synced with what's going on with poco:

https://github.com/pocoproject/poco/network

@aleks-f
Copy link
Member

aleks-f commented Nov 13, 2012

And, when I look better, XML and INI configurations are not #ifdef'd as well.
Günter: is there a reason for it? It doesn't seem to make sense to compile them in and then strip them off Application

@obiltschnig
Copy link
Member

The reason why it's only stripped off Application and not the entire Util library is that when building for smaller embedded Linux systems, we generally link statically. Therefore, it does not hurt if XMLConfiguration gets compiled into the library. It only hurts if it's referenced by Application, because if so, as soon as Application or ServerApplication is used, the entire XML library gets pulled in. This allows us to keep final application binary size around 2MB (when combined with the other --poquito left outs). Back when I did this I did not care about shared libraries, or whether XMLConfiguration actually is in the library. It was just being referenced by Application that mattered.

Günter

Günter Obiltschnig
Applied Informatics Software Engineering GmbH
A-9182 Maria Elend | Maria Elend 96/4 | www.appinf.com
P: +43 4253 32596 M: +43 676 5166737 F: +43 4253 32096

Company Registration: FN 276491 f | Landesgericht Klagenfurt
Managing Director: DI Günter Obiltschnig

On 14.11.2012, at 00:55, Aleksandar Fabijanic wrote:

And, when I look better, XML and INI configurations are not #ifdef'd as well.
Günter: is there a reason for it? It doesn't seem to make sense to compile them in and then strip them off Application


Reply to this email directly or view it on GitHub.

@aleks-f
Copy link
Member

aleks-f commented Nov 14, 2012

I understand. I still think it would be good to strip them all out. Quite often, I find the Util dependency on XML and JSON annoying. Any objections to that?

@obiltschnig
Copy link
Member

No objections

-guenter

Günter Obiltschnig
Applied Informatics Software Engineering GmbH
A-9182 Maria Elend | Maria Elend 96/4 | www.appinf.com
P: +43 4253 32596 M: +43 676 5166737 F: +43 4253 32096

Company Registration: FN 276491 f | Landesgericht Klagenfurt
Managing Director: DI Günter Obiltschnig

On 14.11.2012, at 14:32, Aleksandar Fabijanic wrote:

I understand. I still think it would be good to strip them all out. Quite often, I find the Util dependency on XML and JSON annoying. Any objections to that?


Reply to this email directly or view it on GitHub.

@naquin
Copy link
Author

naquin commented Nov 14, 2012

I will see about putting a separate #ifdef around the XMLConfiguration and JSONConfiguration files then. This should help avoid some confusion on what gets linked in by the application.

@aleks-f
Copy link
Member

aleks-f commented Nov 14, 2012

ok. I'll close this pull request. please fork off the develop branch and send pull request back.

@aleks-f aleks-f closed this Nov 14, 2012
kostya-lnk-ms pushed a commit to kostya-lnk-ms/poco that referenced this pull request Jul 15, 2015
…stream:ms-develop to ms-develop

* commit 'a76079bc7793d32f0943730ff6fb7aa4e028bb7a':
  Removed unused include
@tenglong88 tenglong88 mentioned this pull request Oct 27, 2017
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