Re: [devel] [git update] packages/hasher: heads/arm-port

Re: [devel] [git update] packages/hasher: heads/arm-port

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1146 bytes --]

Hi,

On Sun, Jun 10, 2007 at 06:49:17PM +0400, Kirill A. Shutemov wrote:
> Update of /people/kas/packages/hasher.git
> 
> New head `arm-port' is available with the following commits:
> commit d8f48255dc1595796553bd6dde83ee3c62273de5
> Author: Kirill A. Shutemov <k.shutemov@gmail>
> Date:   Sat May 5 16:51:25 2007 +0259
> 
>     qemu support

Я бы предпочёл более развёрнутый комментарий, сопровождающий коммит.
Степень развёрнутости можно оценить по моим комментариям в этом проекте.
По сути изменений, у меня есть несколько легко поправимых замечаний:

+ Имя переменной qemu слишком коротко для глобальной переменной
конфигурации и потому слишком многозначно; предлагаю изменить на
qemu_path.
+ Глобальная переменная qemu не объявлена в hsh-sh-functions; см. напр.
объявление переменной no_stuff.
+ Добавляемый в hsh-mkchroot код
if [ ! -z "$qemu" ]; then
	install -p -m755 $verbose "$qemu" .host/
fi
вызывает 2 вопроса: зачем написано "! -z" и почему имя создаваемого файла
на выходе зависит от архитектуры? предлагаю изменить на
[ -z "$qemu_path" ] ||
	install -p -m755 $verbose "$qemu" .host/qemu


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [devel] [git update] packages/hasher: heads/arm-port

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 2156 bytes --]

On [Tue, 19.06.2007 12:51], Dmitry V. Levin wrote:
> Hi,
> 
> On Sun, Jun 10, 2007 at 06:49:17PM +0400, Kirill A. Shutemov wrote:
> > Update of /people/kas/packages/hasher.git
> > 
> > New head `arm-port' is available with the following commits:
> > commit d8f48255dc1595796553bd6dde83ee3c62273de5
> > Author: Kirill A. Shutemov <k.shutemov@gmail>
> > Date:   Sat May 5 16:51:25 2007 +0259
> > 
> >     qemu support
> 
> Я бы предпочёл более развёрнутый комментарий, сопровождающий коммит.
> Степень развёрнутости можно оценить по моим комментариям в этом проекте.

Поправлю.

> По сути изменений, у меня есть несколько легко поправимых замечаний: 
> + Имя переменной qemu слишком коротко для глобальной переменной
> конфигурации и потому слишком многозначно; предлагаю изменить на
> qemu_path.

Согласен.

> + Глобальная переменная qemu не объявлена в hsh-sh-functions; см. напр.
> объявление переменной no_stuff.

Ok. Поправлю.

> + Добавляемый в hsh-mkchroot код
> if [ ! -z "$qemu" ]; then
> 	install -p -m755 $verbose "$qemu" .host/
> fi
> вызывает 2 вопроса: зачем написано "! -z"

Поправлю на -n.

>и почему имя создаваемого файла
> на выходе зависит от архитектуры? предлагаю изменить на
> [ -z "$qemu_path" ] ||
> 	install -p -m755 $verbose "$qemu" .host/qemu

Этот скопированный qemu будет использоваться через binfmt. Записей в binfmt
может быть несколько для разных архитектур.

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + Velesys LLC, http://www.velesys.com/
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [devel] [git update] packages/hasher: heads/arm-port

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 1577 bytes --]

On [Tue, 19.06.2007 12:51], Dmitry V. Levin wrote:
> Hi,
> 
> On Sun, Jun 10, 2007 at 06:49:17PM +0400, Kirill A. Shutemov wrote:
> > Update of /people/kas/packages/hasher.git
> > 
> > New head `arm-port' is available with the following commits:
> > commit d8f48255dc1595796553bd6dde83ee3c62273de5
> > Author: Kirill A. Shutemov <k.shutemov@gmail>
> > Date:   Sat May 5 16:51:25 2007 +0259
> > 
> >     qemu support
> 
> Я бы предпочёл более развёрнутый комментарий, сопровождающий коммит.
> Степень развёрнутости можно оценить по моим комментариям в этом проекте.
> По сути изменений, у меня есть несколько легко поправимых замечаний:
> 
> + Имя переменной qemu слишком коротко для глобальной переменной
> конфигурации и потому слишком многозначно; предлагаю изменить на
> qemu_path.
Тут, похоже, получается две переменные: 
qemu_arch сейчас qemu в hsh
qemu_path получается после "$(opt_check_read "$1" "/usr/bin/qemu-$2.static")"

P.S. Мне соит перезаписать историю в git или эти изменения отдельным
коммитом?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + Velesys LLC, http://www.velesys.com/
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [devel] [git update] packages/hasher: heads/arm-port

[Dmitry V. Levin]

[-- Attachment #1: Type: text/plain, Size: 1360 bytes --]

On Tue, Jun 19, 2007 at 03:57:12PM +0300, Kirill A. Shutemov wrote:
> On [Tue, 19.06.2007 12:51], Dmitry V. Levin wrote:
> > On Sun, Jun 10, 2007 at 06:49:17PM +0400, Kirill A. Shutemov wrote:
> > > Update of /people/kas/packages/hasher.git
> > > 
> > > New head `arm-port' is available with the following commits:
> > > commit d8f48255dc1595796553bd6dde83ee3c62273de5
> > > Author: Kirill A. Shutemov <k.shutemov@gmail>
> > > Date:   Sat May 5 16:51:25 2007 +0259
> > > 
> > >     qemu support
> > 
> > Я бы предпочёл более развёрнутый комментарий, сопровождающий коммит.
> > Степень развёрнутости можно оценить по моим комментариям в этом проекте.
> > По сути изменений, у меня есть несколько легко поправимых замечаний:
> > 
> > + Имя переменной qemu слишком коротко для глобальной переменной
> > конфигурации и потому слишком многозначно; предлагаю изменить на
> > qemu_path.
> Тут, похоже, получается две переменные: 
> qemu_arch сейчас qemu в hsh
> qemu_path получается после "$(opt_check_read "$1" "/usr/bin/qemu-$2.static")"

Две переменные -- это перебор и возможная причина конфликта переменных.
Надо придумать способ уместить эту сущность в одну переменную.

> P.S. Мне соит перезаписать историю в git или эти изменения отдельным
> коммитом?

Я ещё не делал merge, поэтому при желании можно перезаписать.


-- 
ldv

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [devel] [git update] packages/hasher: heads/arm-port

[Kirill A. Shutemov]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On [Tue, 19.06.2007 17:33], Dmitry V. Levin wrote:
> On Tue, Jun 19, 2007 at 03:57:12PM +0300, Kirill A. Shutemov wrote:
> > P.S. Мне соит перезаписать историю в git или эти изменения отдельным
> > коммитом?
> 
> Я ещё не делал merge, поэтому при желании можно перезаписать.

Перезаписал. Смотрите ca290a3.
Ещё замечания будут?

-- 
Regards,  Kirill A. Shutemov
 + Belarus, Minsk
 + Velesys LLC, http://www.velesys.com/
 + ALT Linux Team, http://www.altlinux.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]