TheCoder4eu / BootsFaces-OSP

BootsFaces - Open Source Project
Apache License 2.0
246 stars 102 forks source link

Misleading web.xml context parameters #421

Closed zhedar closed 5 years ago

zhedar commented 8 years ago

Due to #414 I just stumbled over our context params and their naming confused me a lot.

The docs state clearly:

Alternatively, you can configure your web.xml to suppress certain resources:

Setting the context parameter net.bootsfaces.get_fontawesome_from_cdn to "yes" or "true" allows you to provide your own Font Awesome file. Setting the context parameter net.bootsfaces.get_jquery_from_cdn to "yes" or "true" allows you to provide your own jQuery file. Setting the context parameter net.bootsfaces.get_jqueryui_from_cdn to "yes" or "true" allows you to provide your own jQueryUI file. Setting the context parameter net.bootsfaces.get_bootstrap_from_cdn to "yes" or "true" allows you to provide your own Bootstrap CSS file. Note that this option is a bit difficult to get up and running.

The context parameter suggests, that we load those components via CDN, if the param is set to true or yes. But we only do that in case of FontAwesome. In fact, we're only getting FA via CDN and don't provide our own, so this is even more confusing.

For the other libraries, we just won't include them if their parameter is set.

I would like to rename those parameters, but that would break backwards compatbility. Maybe we should introduce another parameter, which does the same and is called something like net.bootsfaces.include_jquery, ... and keep both parameters for a couple of minor versions.

What do you think? Was the CDN import ever functioning like the parameter name indicated?

asterd commented 8 years ago

Hi @zhedar, I agree with your point of view. In fact, I ever saw this parameter as "you have to load jquery by yourself, using CDN or whatever you want" (but I usually think in a very strange way :) ), but is true that this name can be misunderstood. So, I like the idea to create two parameters and to provide automatically the CDN URL for JQuery, but in the latter case, we need to choose which version to add (1.x or 2.x??). But, if we decide to add two configuration option, I think is better to maintain them over the time, one to enable real custom settings and one that use a CDN URL of the currently supported version (maybe 3.0 asap!)

zhedar commented 8 years ago

In fact, I ever saw this parameter as "you have to load jquery by yourself, using CDN or whatever you want

Well, if you read the documentation properly, it's properly less confusing :)

I like the idea of providing a CDN url, we could even use the existing the parameter (with it's old functionality still in place) and then transititon to a second parameter. However, if we decide to add an CDN via a URL, we should think about that parameter name again. It would let the user choose, which version to take and to make sure it's compatible with BootsFaces.

stephanrauh commented 5 years ago

While it's true that the names are very confusing, I don't think we should change them after such a long time. Let's close the ticket.