On Tue, Jun 23, 2015 at 5:42 PM, Mike Blumenkrantz <[email protected]> wrote: > On Tue, Jun 23, 2015 at 12:24 PM, Daniel Kolesa <[email protected]> > wrote: > >> On Tue, Jun 23, 2015 at 5:16 PM, Mike Blumenkrantz >> <[email protected]> wrote: >> > It would be great if, in the future, you could refrain from reverts like >> > this. An ongoing mailing list discussion was taking place, and, >> regardless >> > of what you feel "should never be done", now the issue is once again >> > unsolved. >> >> In my experience if a bad solution is left in the tree for too long, >> it'll remain there forever, thus I choose to revert it now. This is >> one of the most awful pieces of code I've seen in the EFL yet and I >> couldn't sleep at night if it was left in, so at least this allows me >> to have a peace of mind; the issue was never really "solved" in the >> first place anyway - touching internal symbols within libraries is a >> big no (I'm sure most of other developers here will agree with my >> statement) and I would appreciate if you discussed things like this on >> the ML before actually introducing them. >> >> Regards, >> >> D5 >> >> > I think this is a very misleading reply. Ignoring the obvious (and quite > unnecessary) use of hyperbole, there are plenty of ways to ensure that code > doesn't remain in the tree forever. I'll list a few of them so that you can > use it as a reference in the event that you have this urge to revert again > in the future: > > * Participate in ongoing mailing list discussions > * Open a ticket on the bug tracker > * Engage in discussion on IRC
It's funny it's you who's telling me this... > > Your claim that the issue was not solved is also inaccurate. This is > exactly how the symbol is accessed in other parts of the tree, You sure? http://pastebin.com/UVUdVuS2 The symbol is only accessed within efreet, which is fine. I'm talking about cross-library internal symbol accesses, which IS bad, and it's not being done anywhere in the EFL. > and having > this ability is one of the great benefits of the unified source tree that > we have. Considering that Edje already depends on Efreet for function > calls, the worst that could happen here is that calling edje_init() at some > point in the future causes a crash--something which would be instantly > noticeable since a general efl tree build would then fail. That doesn't matter. Using internal symbols across libraries like this is bad by definition and should not be encouraged. > > If you can provide me with a comprehensive list of "things like this" which > bother you when they are committed, I can try to initiate discussion in the > community for future cases. That's ridiculous... that this is an ugly hack should be common sense, and if you're unsure about how to make it non-hacky, just initiate a discussion and we can try to work it out some other way. > > >> > >> > On Tue, Jun 23, 2015 at 5:22 AM, Daniel Kolesa <[email protected]> >> wrote: >> > >> >> q66 pushed a commit to branch master. >> >> >> >> >> >> >> http://git.enlightenment.org/core/efl.git/commit/?id=d9db8888ac51175e350853597b3b317e9774a00a >> >> >> >> commit d9db8888ac51175e350853597b3b317e9774a00a >> >> Author: Daniel Kolesa <[email protected]> >> >> Date: Tue Jun 23 10:22:36 2015 +0100 >> >> >> >> Revert "edje: unset efreet cache update flag to prevent dbus >> >> connections" >> >> >> >> This reverts commit 1edb35fff3fe54ac7eea1ba2c26e509284b4e470. >> >> >> >> Accessing symbols from other libs like this should never be done. >> The >> >> consequences of this are not significant enough to do this; better >> >> solution >> >> can be found but definitely not like this. >> >> --- >> >> src/lib/edje/edje_main.c | 2 -- >> >> 1 file changed, 2 deletions(-) >> >> >> >> diff --git a/src/lib/edje/edje_main.c b/src/lib/edje/edje_main.c >> >> index a891ec5..03c46ea 100644 >> >> --- a/src/lib/edje/edje_main.c >> >> +++ b/src/lib/edje/edje_main.c >> >> @@ -1,6 +1,5 @@ >> >> #include "edje_private.h" >> >> >> >> -extern int efreet_cache_update; >> >> static Edje_Version _version = { VMAJ, VMIN, VMIC, VREV }; >> >> EAPI Edje_Version * edje_version = &_version; >> >> >> >> @@ -70,7 +69,6 @@ edje_init(void) >> >> goto shutdown_eet; >> >> } >> >> >> >> - efreet_cache_update = 0; >> >> if (!efreet_init()) >> >> { >> >> ERR("Efreet init failed"); >> >> >> >> -- >> >> >> >> >> >> >> > >> ------------------------------------------------------------------------------ >> > Monitor 25 network devices or servers for free with OpManager! >> > OpManager is web-based network management software that monitors >> > network devices and physical & virtual servers, alerts via email & sms >> > for fault. Monitor 25 devices for free with no restriction. Download now >> > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o >> > _______________________________________________ >> > enlightenment-devel mailing list >> > [email protected] >> > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >> >> >> ------------------------------------------------------------------------------ >> Monitor 25 network devices or servers for free with OpManager! >> OpManager is web-based network management software that monitors >> network devices and physical & virtual servers, alerts via email & sms >> for fault. Monitor 25 devices for free with no restriction. Download now >> http://ad.doubleclick.net/ddm/clk/292181274;119417398;o >> _______________________________________________ >> enlightenment-devel mailing list >> [email protected] >> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel >> > ------------------------------------------------------------------------------ > Monitor 25 network devices or servers for free with OpManager! > OpManager is web-based network management software that monitors > network devices and physical & virtual servers, alerts via email & sms > for fault. Monitor 25 devices for free with no restriction. Download now > http://ad.doubleclick.net/ddm/clk/292181274;119417398;o > _______________________________________________ > enlightenment-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
