Добавил определение установленной версии платформі под linux #39
Conversation
Т.к. в linux нет возмжности определить версию по папке, то используем утилиту rac для получения версии установленной программы, т.к. это базовая зависимость при установке клиента, то всегда должна присутствовать в системе.
src/v8runner.os
Outdated
| ФайлВывода = ВременныеФайлы.НовоеИмяФайла(); | ||
| СтрокаЗапуска = ФайлРАК.ПолноеИмя + " -v > " + ФайлВывода; | ||
| СтрокаЗапуска = "sh -c '" + СтрокаЗапуска + "'"; | ||
| ЗапуститьПриложение(СтрокаЗапуска, , Истина); |
There was a problem hiding this comment.
Предлагаю юзать класс Команда из 1commands, код станет попроще.
There was a problem hiding this comment.
Точнее, код станет намного проще :)
There was a problem hiding this comment.
Ну вся библиотека идет без зависимости commands и вроде как ради одной строчки добавлять зависимость неохота, тем более, что по количесву строк оно проще не станет.
src/v8runner.os
Outdated
| Чтение.Закрыть(); | ||
| мВерсияПлатформы = СокрЛП(Текст); | ||
| Исключение | ||
| Лог.Предупреждение("Не удалось прочитать файл версии 1С %1, %2. |
There was a problem hiding this comment.
А в чем проблема с чтением результата вывода?
неужели есть какой-то сценарий?
|
Любое io должно быть завернуто в попытку. Так что она тут вполне заслуженно |
artbear
left a comment
artbear
left a comment
There was a problem hiding this comment.
Женя, еще
- нужно добавить зависимость от 1commands в packagedef
- еще дополнения
src/v8runner.os
Outdated
| мВерсияПлатформы = СокрЛП(Команда.ПолучитьВывод()); | ||
| Исключение | ||
| Лог.Предупреждение("Не удалось прочитать версию 1С %1, %2. | ||
| |" + ОписаниеОшибки(), мВерсияПлатформы, СтрокаЗапуска); |
There was a problem hiding this comment.
Мы внутри исключения используем переменную мВерсияПлатформы, которую устанавливаем внутри блока Попытка.
Это плохо, ведь переменная может быть пустой.
ИМХО в сообщение ошибки нужно вставить параметр метода ВерсияПлатформы вместо мВерсияПлатформы
There was a problem hiding this comment.
Предлагаю расширить сообщение ошибки, т.к. сейчас оно не поможет пользователю понять проблему
Лог.Предупреждение("Не удалось прочитать версию 1С %1, %2.
|%3
|%4", ВерсияПлатформы, СтрокаЗапуска, Команда.ПолучитьВывод(),
ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()) );There was a problem hiding this comment.
мВерсияПлатформы - итак неопределено, еще ранее, оно или определяется или нет. Зачем кидать в какие-то переменные, тем более в самом конце попытки только и происходит присвоение значение переменной, это вообще никакой пользы не принесет дополнительное присваивание.
Команда.ПолучитьВывод() будет в ИнформацияОбОшибке() я как раз для этого и обновлял 1ccommands
There was a problem hiding this comment.
Насчет мВерсияПлатформы не согласен.
Подумай над сценарием
в линуксе мВерсияПлатформы пустая
далее падает Команда.Исполнить
в попытке мы выводим сообщение Не удалось прочитать версию 1С , folder\rac -v + сообщение ошибки.
и показа версии все равно не будет.
а пользователю было бы удобнее получить сообщение Не удалось прочитать версию 1С 8.3, folder\rac -v вместо вышеуказанного
There was a problem hiding this comment.
я добавил УстановитьПравильныйКодВозврата(0) в случаи ошибки до присваивания даже не дойдет. Зачем писать какой-то мифический 8.3 если завтра будет 8.4 и т.д.?
There was a problem hiding this comment.
Я и говорю, что до присваивания дело не дойдет.
И тогда нет смысла использовать пустую переменную.
Про 8.3 - я не предлагаю вставить жестко магическую строку "8.3" :(
я говорил о том, что будет, когда клиент библиотеки вызовет обсуждаемый метод
ПолучитьПутьКВерсииПлатформы("8.3");ПолучитьПутьКВерсииПлатформы("8.4");ПолучитьПутьКВерсииПлатформы("9.0");
в этом случае в случае ошибок ему было бы удобно получить не просто "пустую" строку ошибки, без искомой версии, а удобно получить более развернутое сообщение с искомой версией, т.е. с 8.3, 8.4 или 9.0
There was a problem hiding this comment.
Я писал о вставке параметра метода ВерсияПлатформы, в котором и будет 8.3, 8.4 или 9.0
There was a problem hiding this comment.
А вообще это мелочь, конечно.
Если считаешь, что все нормально, можешь и замержить.
| Если ФайлРАК.Существует() Тогда | ||
| Команда = Новый Команда; | ||
| СтрокаЗапуска = ФайлРАК.ПолноеИмя + " -v "; | ||
| Команда.УстановитьСтрокуЗапуска(СтрокаЗапуска); |
There was a problem hiding this comment.
Предлагаю еще добавить очень полезную настройку Команда.УстановитьПравильныйКодВозврата(0);
Тогда будет выброшено исключение, если получим другой код возврата.
There was a problem hiding this comment.
Нафиг, это попытка, там может и неправильный код. А исключение по УстановитьПравильныйКод - вообще неиформативно, "Ожидали 0 вместо 1 ", а что там по факту не так никогда не узнаешь.
|
Разве 0 - не дефолтный код возврата? |
|
П.С. С телефона и не могу комментировать инлайн |
|
По умолчанию проверка кода возврата выключена, т.к. это поведение не всегда нужно.
|
|
Подстава... @khorevaa |
|
А я вам в PR гитраннера писал об этом :) |
|
Больше возражений, кроме использования мВерсияПлатформы, нет. |
| Попытка | ||
| Команда.Исполнить(); | ||
| мВерсияПлатформы = СокрЛП(Команда.ПолучитьВывод()); | ||
| ВерсияПлатформы = СокрЛП(Команда.ПолучитьВывод()); |
There was a problem hiding this comment.
В итоге все равно ИМХО неверно :(
Зачем заменять передаваемый в функцию параметр ВерсияПлатформы на какой-то вычисляемый??
3 participants
Т.к. в linux нет возмжности определить версию по папке, то используем утилиту rac для получения версии установленной программы, т.к. это базовая зависимость при установке клиента, то всегда должна присутствовать в системе.