Warn against overcomplicated logic in config plugins

[Grinnz]

Summary

Include a warning in the two config plugins against improper use of their embedded Perl convenience features.

Motivation

Embedded Perl in configuration can be dangerous if misused, and a maintenance burden if overused.

[kraih]

Do we warn against overcomplicated logic in templates too?

[Grinnz]

We do not -- I think it's less of a concern, and there are far more valid uses of logic in templates like helpers, but a brief mention in https://metacpan.org/pod/Mojolicious::Guides::Rendering#Embedded-Perl may be worthwhile.

[jhthorsen]

I do think that this should be general knowledge in any context, but at the same time it doesn't hurt mentioning it I. I'm neutral to this change.

[Grinnz]

Calling for a vote, or any suggestions to wording @mojolicious/core

[jberger]

I kinda feel like once you know that the config file is executed, warning of additional logic is more "at your own risk" rather than discouraged. Like @jhthorsen said, it should be general knowledge, IMO. That said, I won't block it either. I guess I'm also neutral.

[Grinnz]

I kinda feel like once you know that the config file is executed, warning of additional logic is more "at your own risk" rather than discouraged.

I kind of disagree, while not always explicitly the framework makes an effort to discourage bad practices.

[christopherraa]

I don't have voting rights, but I thought I'd mention that I am torn on this issue myself. Although I have seen (and had to clean up) absurd and excessive logic in configuration files I feel that this i perhaps a question of training, methodology and policy. In the last case I encountered this issue I spoke to the developer and it turned out that he simply did not see another solution (which I then showed him).

If warnings were to be added I would definitly want the warnings to be prominent, but only visible in development mode, just so that we do not end up with warnings in production-logs where there are possible legitimate use cases for logic in the configuration.

I guess that means I'm neutral.

[marcusramberg]

I think this is a good warning, as it’s less obvious than with templates. I’m 👍

[Grinnz]

To be clear this isn't a warning in code, just documentation.

[CandyAngel]

while not always explicitly the framework makes an effort to discourage bad practices.

Maybe these sort of things should all be documented in one place (e.g. Mojolicious::Guides::Pitfalls) with links to that from the affected modules descriptions (or specific methods if applicable). "Overcomplicated" configuration files, empty strings in paths with Mojo::File and other potential traps and bad practices could be enumerated there.

Aside from collating everything, the Guides are also more "prose-y", differing from the succinct technical description of the module documentation (which in this case, might just contain a note that it is "evaluated Perl" as an implementation detail). This means they can contain more details of what is considered "overcomplicated" and why it is considered bad (with numerous examples if required) and advice on alternative, better solutions.

Still, being warned of potential dangers is always useful, so I'm voting "yes" to this change.

[s1037989]

That's a great idea, @CandyAngel ! Almost the opposite of a Cookbook. :) I find that there are often things that I do that I later learn aren't best practice or are considered bad ideas... And the reality is I just didn't know. I do my best to do my best, but when you don't know what you don't know, it can make it really hard sometimes. I think so many people would benefit from a guide like you suggest. Mojolicious is such an awesome framework, and it isn't just for experts. A document to help bridge that gap would really help a lot of people and bring down a lot of barriers to entry, I think.

[kraih]

I'm afraid this vote has been invalidated by a rule change and needs to be redone. 41d1d1c...b4152fd

[kraih]

I vote neutral.

[marcusramberg]

I am 👍 on this change.

[jberger]

As stated before, I don't like discouraging actions when the reasoning has already been made known. I guess that though I like the idea of informing the user that these config files are executed I'm a net negative on this PR as is.

[kiwiroy]

In the last case I encountered this issue I spoke to the developer and it turned out that he simply did not see another solution (which I then showed him).

@christopherraa this make me think that there should be a link, or further explanation section, possibly with an example. If you find yourself including code, consider this/these options instead ...

[assistcontrol]

As an end-user, I'd really love a "best practices" document from the people who know Mojo best. Nowhere else do Mojo docs really say that something works but is bad design.

[kraih]

I'm afraid the 14 day review period has passed and this pull request did not reach the required number of votes.