FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Gentoo > Gentoo Development

 
 
LinkBack Thread Tools
 
Old 07-05-2010, 09:38 PM
Arfrever Frehtes Taifersar Arahesis
 
Default Minor changes in python.eclass and distutils.eclass

2010-07-05 23:28:01 Samuli Suominen napisał(a):
> On 07/05/2010 11:23 PM, Arfrever Frehtes Taifersar Arahesis wrote:
> > 2010-07-05 21:18:57 Mark Loeser napisał(a):
> >> Arfrever Frehtes Taifersar Arahesis <Arfrever@gentoo.org> said:
> >>> 2010-07-05 20:00:11 Mark Loeser napisał(a):
> >>>> Everyone else has already made valid points. I'm just picking this one
> >>>> to reply to now. Please remove the colors you have added. If you need
> >>>> a new function, say "eqawarn", we should have that added in the next
> >>>> EAPI with a description of when and where to use it.
> >>>
> >>> In case of the colored message added in this patch, if einfo/elog/ewarn/eqawarn/eerror was used,
> >>> then its output wouldn't be logged by Portage.
> >>
> >> I don't understand what you are trying to say.
> >
> > Portage doesn't log output of einfo/elog/ewarn/eqawarn/eerror called in global scope.
>
> Can you point to the location of python.eclass / distutils.eclass where
> you need to send output from global scope, please?

It's at the beginning of this patch. NEED_PYTHON is parsed in global scope.

--
Arfrever Frehtes Taifersar Arahesis
 
Old 07-05-2010, 09:49 PM
Samuli Suominen
 
Default Minor changes in python.eclass and distutils.eclass

On 07/06/2010 12:38 AM, Arfrever Frehtes Taifersar Arahesis wrote:
> 2010-07-05 23:28:01 Samuli Suominen napisał(a):
>> On 07/05/2010 11:23 PM, Arfrever Frehtes Taifersar Arahesis wrote:
>>> 2010-07-05 21:18:57 Mark Loeser napisał(a):
>>>> Arfrever Frehtes Taifersar Arahesis <Arfrever@gentoo.org> said:
>>>>> 2010-07-05 20:00:11 Mark Loeser napisał(a):
>>>>>> Everyone else has already made valid points. I'm just picking this one
>>>>>> to reply to now. Please remove the colors you have added. If you need
>>>>>> a new function, say "eqawarn", we should have that added in the next
>>>>>> EAPI with a description of when and where to use it.
>>>>>
>>>>> In case of the colored message added in this patch, if einfo/elog/ewarn/eqawarn/eerror was used,
>>>>> then its output wouldn't be logged by Portage.
>>>>
>>>> I don't understand what you are trying to say.
>>>
>>> Portage doesn't log output of einfo/elog/ewarn/eqawarn/eerror called in global scope.
>>
>> Can you point to the location of python.eclass / distutils.eclass where
>> you need to send output from global scope, please?
>
> It's at the beginning of this patch. NEED_PYTHON is parsed in global scope.
>

OK, so let me get this right... are trying to justify using all of the
echo+custom colorization in the eclass by this one occurance, in basis
of it doesn't get logged otherwise?

Seems a bit far fetch. Also I'm not convinced if it's beneficial to log
this.
 
Old 07-05-2010, 10:07 PM
Arfrever Frehtes Taifersar Arahesis
 
Default Minor changes in python.eclass and distutils.eclass

2010-07-05 23:49:58 Samuli Suominen napisał(a):
> On 07/06/2010 12:38 AM, Arfrever Frehtes Taifersar Arahesis wrote:
> > 2010-07-05 23:28:01 Samuli Suominen napisał(a):
> >> On 07/05/2010 11:23 PM, Arfrever Frehtes Taifersar Arahesis wrote:
> >>> 2010-07-05 21:18:57 Mark Loeser napisał(a):
> >>>> Arfrever Frehtes Taifersar Arahesis <Arfrever@gentoo.org> said:
> >>>>> 2010-07-05 20:00:11 Mark Loeser napisał(a):
> >>>>>> Everyone else has already made valid points. I'm just picking this one
> >>>>>> to reply to now. Please remove the colors you have added. If you need
> >>>>>> a new function, say "eqawarn", we should have that added in the next
> >>>>>> EAPI with a description of when and where to use it.
> >>>>>
> >>>>> In case of the colored message added in this patch, if einfo/elog/ewarn/eqawarn/eerror was used,
> >>>>> then its output wouldn't be logged by Portage.
> >>>>
> >>>> I don't understand what you are trying to say.
> >>>
> >>> Portage doesn't log output of einfo/elog/ewarn/eqawarn/eerror called in global scope.
> >>
> >> Can you point to the location of python.eclass / distutils.eclass where
> >> you need to send output from global scope, please?
> >
> > It's at the beginning of this patch. NEED_PYTHON is parsed in global scope.
> >
>
> OK, so let me get this right... are trying to justify using all of the
> echo+custom colorization in the eclass by this one occurance, in basis
> of it doesn't get logged otherwise?

I was justifying using colors in this case (NEED_PYTHON). I'm completely against not using colors
in status messages (e.g. "Building of dev-python/setuptools-0.6.13 with CPython 3.1..."), which
are more useful, when they are easily noticeable by ebuild maintainers. I might agree to remove
colors in other deprecation warnings, but this patch wasn't adding any other deprecation warnings.
Not using colors in deprecation warnings doesn't have any benefits and only decreases noticeability
of deprecation warnings.

--
Arfrever Frehtes Taifersar Arahesis
 
Old 07-05-2010, 10:29 PM
Mike Frysinger
 
Default Minor changes in python.eclass and distutils.eclass

On Monday, July 05, 2010 16:23:50 Arfrever Frehtes Taifersar Arahesis wrote:
> 2010-07-05 21:18:57 Mark Loeser napisał(a):
> > Arfrever Frehtes Taifersar Arahesis said:
> > > 2010-07-05 20:00:11 Mark Loeser napisał(a):
> > > > Everyone else has already made valid points. I'm just picking this
> > > > one to reply to now. Please remove the colors you have added. If
> > > > you need a new function, say "eqawarn", we should have that added in
> > > > the next EAPI with a description of when and where to use it.
> > >
> > > In case of the colored message added in this patch, if
> > > einfo/elog/ewarn/eqawarn/eerror was used, then its output wouldn't be
> > > logged by Portage.
> >
> > I don't understand what you are trying to say.
>
> Portage doesn't log output of einfo/elog/ewarn/eqawarn/eerror called in
> global scope.

dont call it in global scope then. the more parsing you do in global scope
the worse you make performance for the tree. these things get executed during
dependency generation which means it gets spammed even when not emerging.

use pkg_setup or something similar like everyone else.
-mike
 
Old 07-05-2010, 10:50 PM
"Jorge Manuel B. S. Vicetto"
 
Default Minor changes in python.eclass and distutils.eclass

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 05-07-2010 15:23, Arfrever Frehtes Taifersar Arahesis wrote:
> These minor changes in python.eclass and distutils.eclass have been already
> reviewed on alias of Gentoo Python Project. It's recommended to be familiar
> with internals of current code before trying to understand these minor changes.
> Suggestions about indentation and quoting will be rejected.

Arfrever,

I'm not going to delve into the details that have been addressed all
other this thread. Instead I'll just address one small issue.
The use of *minor* in the title of this thread and the sheer size of the
patch attached are not compatible. Please don't label changes such as
these as *minor* in future instances.


- --
Regards,

Jorge Vicetto (jmbsvicetto) - jmbsvicetto at gentoo dot org
Gentoo- forums / Userrel / Devrel / KDE / Elections
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.15 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBAgAGBQJMMmHQAAoJEC8ZTXQF1qEPzG4P/RMaVOxp/cCTq5Y5RZQq0IIE
t6nh8AJGl0nJyjDi4LtpI2A1BpC2m0EcekDmYLkQouo9+IRwIr bawI4N8MQFAiZC
Nj/yideygJMFYE7o6RiIxZ3HtCxhoV0uY48c0azxehwVBykrLKx2G NLLvmveqGcj
85saWl0awP9/KktYYUt+EHTBsTmJCmQX9493pxWUrMczFrz+eyDMjgDc9La77S uU
u3JEFLLmvHtLgY8rAoSm3zDulxsc+fmdWTsS1/36Ko+StAQLMDz1OGUEsokQPSMB
IeouOKxHGIlefaAYhrqtIni+M9hkxFRLS46zmcrD9J36SYcPrD 0XagWtEdb31+Ar
PPssQJnUz4FaVXd4rLVhBdOc2FpZR1VqaMK2wVhYKm1aq+We68 LGxEd5JU0PCPJT
Y9Do3YfolXyfsF/zDkmuq0v2613i7APqVrmfK3kMHn0u8Ytma1JdLD/KzF/AOwTM
WCoH5Q5os59oN0gpbYUx1EHzIq0qfY6IslZLu/cedN6hM9QK+TOWzks2bqa888iI
J6h9kd20FcfddAbbBIfhe0jSk1kRxVYV7RLfvsBZ2FB7PeIKql i+wDBUOj36FmU3
i+W5v5Uu134Dpdn+pevtFeDZsu4bmXrkasg8HvlH7J2oquCu2E gb88ST0dQEB08Z
lNYugouUn/oPIrgk76t5
=qBaG
-----END PGP SIGNATURE-----
 
Old 07-05-2010, 11:18 PM
Jeroen Roovers
 
Default Minor changes in python.eclass and distutils.eclass

On Mon, 05 Jul 2010 22:50:56 +0000
"Jorge Manuel B. S. Vicetto" <jmbsvicetto@gentoo.org> wrote:

> I'm not going to delve into the details that have been addressed all
> other this thread. Instead I'll just address one small issue.
> The use of *minor* in the title of this thread and the sheer size of
> the patch attached are not compatible. Please don't label changes
> such as these as *minor* in future instances.

Another major flaw on its own is the sheer size of the patch (it hadn't
been explicitly mentioned to my knowledge). Aren't we supposed to
commit early and often in open source projects instead of dumping all
your changes on reviewers at once (first fork and later merge)? And if
you're still going to do it, a the huge patch should only be necessary
to establish a single sweeping change, not many small changes.


jer
 
Old 07-05-2010, 11:31 PM
Arfrever Frehtes Taifersar Arahesis
 
Default Minor changes in python.eclass and distutils.eclass

2010-07-06 00:29:09 Mike Frysinger napisał(a):
> On Monday, July 05, 2010 16:23:50 Arfrever Frehtes Taifersar Arahesis wrote:
> > 2010-07-05 21:18:57 Mark Loeser napisał(a):
> > > Arfrever Frehtes Taifersar Arahesis said:
> > > > 2010-07-05 20:00:11 Mark Loeser napisał(a):
> > > > > Everyone else has already made valid points. I'm just picking this
> > > > > one to reply to now. Please remove the colors you have added. If
> > > > > you need a new function, say "eqawarn", we should have that added in
> > > > > the next EAPI with a description of when and where to use it.
> > > >
> > > > In case of the colored message added in this patch, if
> > > > einfo/elog/ewarn/eqawarn/eerror was used, then its output wouldn't be
> > > > logged by Portage.
> > >
> > > I don't understand what you are trying to say.
> >
> > Portage doesn't log output of einfo/elog/ewarn/eqawarn/eerror called in
> > global scope.
>
> dont call it in global scope then. the more parsing you do in global scope
> the worse you make performance for the tree. these things get executed during
> dependency generation which means it gets spammed even when not emerging.
>
> use pkg_setup or something similar like everyone else.

Only 84 ebuilds in the tree set NEED_PYTHON variable. NEED_PYTHON can be used only in EAPI <=2.
python_pkg_setup() is optional in EAPI <=3. Exactly 0 ebuilds, which use NEED_PYTHON, also call
python_pkg_setup(). EXPORT_FUNCTIONS wasn't ommitted during this calculation.

There are over 27000 ebuilds in the tree. Performance penalty from <=84 ebuilds shouldn't be
noticeable during `emerge --regen`.

--
Arfrever Frehtes Taifersar Arahesis
 
Old 07-05-2010, 11:37 PM
Mike Frysinger
 
Default Minor changes in python.eclass and distutils.eclass

On Monday, July 05, 2010 19:31:59 Arfrever Frehtes Taifersar Arahesis wrote:
> Only 84 ebuilds in the tree set NEED_PYTHON variable. NEED_PYTHON can be
> used only in EAPI <=2. python_pkg_setup() is optional in EAPI <=3. Exactly
> 0 ebuilds, which use NEED_PYTHON, also call python_pkg_setup().
> EXPORT_FUNCTIONS wasn't ommitted during this calculation.

well then, it shouldnt be hard for you to fix that small number and drop this
crap from the tree.

your global scope echo still doesnt require color support. everyone is
telling you for obvious reasons to stop doing this. so do it already.

> There are over 27000 ebuilds in the tree. Performance penalty from <=84
> ebuilds shouldn't be noticeable during `emerge --regen`.

your few ebuilds and my few ebuilds and that guys few ebuilds and ... see how
it all adds up ?
-mike
 
Old 07-06-2010, 06:33 AM
Petteri Rty
 
Default Minor changes in python.eclass and distutils.eclass

On 07/06/2010 02:18 AM, Jeroen Roovers wrote:
> On Mon, 05 Jul 2010 22:50:56 +0000
> "Jorge Manuel B. S. Vicetto" <jmbsvicetto@gentoo.org> wrote:
>
>> I'm not going to delve into the details that have been addressed all
>> other this thread. Instead I'll just address one small issue.
>> The use of *minor* in the title of this thread and the sheer size of
>> the patch attached are not compatible. Please don't label changes
>> such as these as *minor* in future instances.
>
> Another major flaw on its own is the sheer size of the patch (it hadn't
> been explicitly mentioned to my knowledge). Aren't we supposed to
> commit early and often in open source projects instead of dumping all
> your changes on reviewers at once (first fork and later merge)? And if
> you're still going to do it, a the huge patch should only be necessary
> to establish a single sweeping change, not many small changes.
>

Indeed the patch should be splitted if you expect people to reasonably
review it.

Petteri
 
Old 07-14-2010, 10:36 PM
Arfrever Frehtes Taifersar Arahesis
 
Default Minor changes in python.eclass and distutils.eclass

2010-07-06 01:37:01 Mike Frysinger napisał(a):
> On Monday, July 05, 2010 19:31:59 Arfrever Frehtes Taifersar Arahesis wrote:
> > Only 84 ebuilds in the tree set NEED_PYTHON variable. NEED_PYTHON can be
> > used only in EAPI <=2. python_pkg_setup() is optional in EAPI <=3. Exactly
> > 0 ebuilds, which use NEED_PYTHON, also call python_pkg_setup().
> > EXPORT_FUNCTIONS wasn't ommitted during this calculation.
>
> well then, it shouldnt be hard for you to fix that small number and drop this
> crap from the tree.
>
> your global scope echo still doesnt require color support. everyone is
> telling you for obvious reasons to stop doing this. so do it already.

NEED_PYTHON has been removed from all ebuilds in gentoo-x86, so now there shouldn't be any
problems with printing this deprecation warning in global scope.

--
Arfrever Frehtes Taifersar Arahesis
 

Thread Tools




All times are GMT. The time now is 05:52 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org