dd/359/do_with_docker_v5 #5
Conversation
|
NB: лучше ссылаться на PR вот так: #4 |
|
Вынеси, пожалуйста, добавление необходимых докеры файлов в отдельный коммит. |
captainenhancement
force-pushed
the
dd/359/do_with_docker_v5
branch
3 times, most recently
from
aef47f8 to
b49f3c2
Compare
|
готово |
|
|
||
| require 'lua-nucleo' -- for import() | ||
|
|
||
| -- for lfs.chdir(), lfs.currentdir() |
There was a problem hiding this comment.
Очень странные манипуляции. Объясни, что тут происходит, пожалуйста.
Откуда берется первый lfs? Почему не использовать конструкцию вида new_lfw = require 'lfs'?
There was a problem hiding this comment.
проблема: когда импортится lfs, он без-условно пишет свою таблицу в глобальную переменную
решение: 1)в переменную orig_lfs сохраняется предыдущее значение lfs (скорее всего, null)
2)require-им lfs
3)в new_lfs пишем то, что установил модуль при загрузке
4)восстанавливаем глобальную lfs её оригинальным, старым значением
создаем для удобства локальную (внутри модуля) lfs, которая выглядит привычно
There was a problem hiding this comment.
Во-первых, в Lua нет null'а, есть nil.
Во-вторых, я не понимаю, почему нельзя просто сделать local lfs = require 'lfs'.
There was a problem hiding this comment.
есть (оставить пока как есть)
|
Дополнительно: мне не нравится, что обертки над LFS находятся в одном файле с кодом docker'а. Конструктивных предложений нет, но читать тяжело. |
captainenhancement
force-pushed
the
dd/359/do_with_docker_v5
branch
from
b49f3c2 to
6f454b4
Compare
|
готово. перезапушено (форспуш) |
| sock:close() | ||
| break | ||
| end | ||
| sock:send(line.."\n") |
test/cases/0030-docker.lua
Outdated
| -------------------------------------------------------------------------------- | ||
|
|
||
| local log, dbg, spam, log_error | ||
| = import 'lua-aplicado/log.lua' { 'make_loggers' } ( |
There was a problem hiding this comment.
"=" должно быть точно под "l" из "log" (подвинуть на 1 вперед), или "=" должно быть на 2 пробела от начала?
| ----------------------------------------------------------------------- | ||
|
|
||
| local log, dbg, spam, log_error | ||
| = import 'lua-aplicado/log.lua' { 'make_loggers' } ( |
There was a problem hiding this comment.
Кажется, по историческим причинам гайдлайны не разрешают писать так импорт в одну строку.
There was a problem hiding this comment.
There was a problem hiding this comment.
Очень странно. Попроси AG прокомментировать
There was a problem hiding this comment.
гайдлайны это разрешают (по крайней мере для логгеров), вот там даже цитата есть
There was a problem hiding this comment.
есть (оставлено в одну строку)
pk-test/docker.lua
Outdated
| 'do_with_docker() is called with arguments: dir =', | ||
| cfg_dir, | ||
| 'handler =', | ||
| handler |
There was a problem hiding this comment.
в одну строку они не поместятся в 80 символов
There was a problem hiding this comment.
есть (сделано в две)
captainenhancement
force-pushed
the
dd/359/do_with_docker_v5
branch
from
6f454b4 to
307f0d3
Compare
|
готово. перезапушено (форспуш) 2 |
same as PR #4 (do_with_docker() feature and it's tests, reformatted according Lua-coding-guidelines) but without devel history
upd:
= no changes in code
= test passes successfully
* 2 commits -> 3 commits