Page MenuHomePhabricator

edje: add getenv for turning on edje nosave
Needs RevisionPublic

Authored by eagleeye on Jan 17 2020, 3:28 AM.

Details

Reviewers
Hermet
cedric
Summary

Sometimes we want to turn on edje nosave option by system side.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 15435
Build 10614: arc lint + arc unit
eagleeye created this revision.Jan 17 2020, 3:28 AM
eagleeye requested review of this revision.Jan 17 2020, 3:28 AM
cedric requested changes to this revision.Jan 17 2020, 10:54 AM
cedric added inline comments.
src/bin/edje/edje_cc_out.c
2228

I think this is a bad idea. If you have a system wide environment variable that turnoff the ability to update theme files and no warning. You might be doing a mistake with no feedback at all on what is going on.

This revision now requires changes to proceed.Jan 17 2020, 10:54 AM

@cedric "turnoff the ability to update theme files" What do you mean by it?

Second question is, what apps supposed to do by this feedback ? Thus they can use no_save or they should not use them?

It looks like irresponsible option nor unnecessary to apps to choose.

I think system could decide the default option like gcc Optimization level.

@cedric "turnoff the ability to update theme files" What do you mean by it?

Second question is, what apps supposed to do by this feedback ? Thus they can use no_save or they should not use them?

Once you remove the source that is included in Edje files, you will now depend on who ever write the application to provide update on the theme. This decision being done at system level needs to be well understood by the developers that is making it. Having an environment variable that disable it system wide means that this can be embedded in a build system to silence the information to the developer working on the app. At this point you have a collision, as the developer might not know he is taking responsibility for maintenance of the theme as he might have not set the environment variable.

It looks like irresponsible option nor unnecessary to apps to choose.

Irresponsibility in this case is if the developer doesn't understand or know his own responsibility in the maintenance of the theme.

I think system could decide the default option like gcc Optimization level.

I am fine with the environment variable defaulting to a change of saving source policy, but the developer must know the responsibility he is taking. I am not sure that even a flag passed to edje_cc would be an acceptable way to silence this warning as this also can be defined silently in the build system.

At the end, the cost of not saving the source in the theme is that who ever package the application become responsible to provide both the source code of the app and the theme as part as his long term maintenance duty. If you think it is not a necessity, then you can go ahead and push this patch.