On Wed, May 9, 2018 at 4:53 PM, Vincent Massol <vinc...@massol.net> wrote:
> Hi, > > > On 9 May 2018, at 15:46, Thomas Mortagne <thomas.morta...@xwiki.com> > wrote: > > > > Hi xwikiers, > > > > Denis expressed to me some concerns about the type that should be > > associated to Mail.MailConfig since it contains configuration data. > > > > One important issue I see if that this page is primarily an admin > > configuration UI > > I don’t understand this sentence. MailConfig is not an UI and it doesn’t > contain configuration for UI. It contains config for the mail server, mail > properties, etc. It contains mail configuration data. Not related to UI. > > > which happen to also contains configuration data > > (would have been much cleaner to store the data in a generated > > document…) > > Same, could you explain this more? > > > so I think the best for now is to keep the default type. > > Since you are not supposed to go trough edit more to modify those > > configuration data, the edit protection should not really affect users > > in practice (no warning when using admin UI). > > We happen to have some Admin UI that changes the MailConfig page so I’m > fine to use ‘default’ (i.e. to warn the users when they try to edit it). > > The important aspect is that we don’t try to merge it when the admin > upgrades the wiki and there’s been changes brought to it (through the Admin > UI), and I believe “default” will achieve this. > Note that on the merge side, the purpose of "default" is to explicitly merge any updates. "default: used to force the default. Edit and delete are not allowed and a 3-way merge is applied to the document during upgrades." Perhaps a more appropriate type for this page would be: "configuration: a document containing configuration. Edit is allowed and the document is never upgraded." ...since, after all, the main purpose of Mail.MailConfig is to be a configuration page, with some stray (and rather static, i.e. don't change often) UI elements in it (ConfigurationClass) that need to be extracted to a different page (and *that* page will be set to type "default"). > > Now we might have other *Config pages for which we’re currently lacking an > Admin UI till we provide such a UI, we’ll need to mark those as “demo” for > now. > Same here... semantically, those are "configuration" pages as well. Side note: Now, I notice a weird thing in the definition of the "configuration" type: "Edit is allowed <note for Vincent: delete is disallowed. We should write this explicitly to clarify> and the document is never upgraded." ...and I kind of understand why Vincent might have suggested "demo", which says: "Edit and delete are allowed and the document is upgraded only if it's not been customized." ...which is something that should, IMO, apply to "configuration" types as well, since it would make sense to keep using the default configuration values of an application (whichever those are and however they evolve across time), until we start customizing them. i.e. "configuration" should be: allow edit, disallow delete and only upgrade if there were no changes. WDYT? Side note2: Just a quick reminder that for configuration, in the best case scenario (where we have no mixing of code with configuration data), the default actions should be to merge, like all the other package managers out there. Thanks, Eduard > > So ok for me for MailConfig (if I understood correctly). > > Thanks > -Vincent > > > > > WDYT ? > > > > -- > > Thomas Mortagne > >
