Skip to content

Conversation

@khorevaa
Copy link
Member

No description provided.

@artbear
Copy link
Member

artbear commented Jan 17, 2017

Что-то табы и пробелы сильно различаются :(

@khorevaa
Copy link
Member Author

Рецепт сделать конвертацию и положить в ветку потом сравнивать. А вообще надо определиться к неиспользованию табов

@nixel2007
Copy link
Member

  1. Не надо #Использовать ..
                                        Знач Права = ПраваПользователяХранилища.ТолькоЧтение,

нельзя использовать непримитивные типы в параметрах процедуры. Если это работает, то это баг в движке.

src/v8runner.os Outdated
// LockObjects — право на захват объектов,
// ManageConfigurationVersions — право на изменение состава версий,
// Administration — право на административные функции.
// RestoreDeletedUser — Если обнаружен удаленный пользователь с таким же именем, он будет восстановлен.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Этого значения нет в перечислении

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ага, RestoreDeletedUser нет в перечислении.

Я предлагаю исправить описание метода, заменив упоминание значение на упоминание ключей перечисления.
Пользователь метода ведь должен работать с ключами (Права = ПраваПользователяХранилища.ТолькоЧтение;, как у тебя же в коде ниже и написано), а значения ему не так интересны.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправил!

УправлениеКонфигуратором.СоздатьФайловоеХранилищеКонфигурации(
КаталогВременногоХранилища,
"Администратор");
Утверждения.ПроверитьИстину(УправлениеКонфигуратором.ХранилищеКонфигурацииСуществует(КаталогВременногоХранилища), "Временное хранилище конфигурации должно существовать");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

У меня сомнения насчет метода ХранилищеКонфигурацииСуществует. Он точно нужен в публичном API пакета? А если так, то он работает только для файловой версии хранилища, с сервером не заработает. Тогда опять вопрос в его полезности. И так по кругу...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно перенести в модуль тестов..

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, нужно перенести этот метод из API

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Перенес в модуль тестов.


Параметры = СтандартныеПараметрыЗапускаКонфигуратора();

Параметры.Добавить("/ConfigurationRepositoryF """+СтрокаСоединения+"""");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Предлагаю след.код

Параметры.Добавить("/ConfigurationRepositoryF """+СтрокаСоединения+"""");
Параметры.Добавить("/ConfigurationRepositoryN """+ПользовательХранилища+"""");
    
Если Не ПустаяСтрока(ПарольХранилища) Тогда
    Параметры.Добавить("/ConfigurationRepositoryP """+ПарольХранилища+"""");
КонецЕсли;

выделить в отдельный метод для исключения его дублирования в нескольких методах работы с хранилищем.

Например, ДобавитьПараметрыПодключенияКХранилищу( Параметры, Знач СтрокаСоединения, Знач ПользовательХранилища, Знач ПарольХранилища)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

выделить в отдельный метод для исключения его дублирования в нескольких методах работы с хранилищем.

Ага, я пропустил #19

src/v8runner.os Outdated
// LockObjects — право на захват объектов,
// ManageConfigurationVersions — право на изменение состава версий,
// Administration — право на административные функции.
// RestoreDeletedUser — Если обнаружен удаленный пользователь с таким же именем, он будет восстановлен.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ага, RestoreDeletedUser нет в перечислении.

Я предлагаю исправить описание метода, заменив упоминание значение на упоминание ключей перечисления.
Пользователь метода ведь должен работать с ключами (Права = ПраваПользователяХранилища.ТолькоЧтение;, как у тебя же в коде ниже и написано), а значения ему не так интересны.

src/v8runner.os Outdated
// ПарольХранилища - Строка - Пароль пользователь для подключения к хранилищю конфигурации
// НовыйПользователь - Строка - Имя создаваемого пользователя.
// ПарольПользователя - Строка - Пароль создаваемого пользователя.
// Права — ПраваПользователяХранилища -Права пользователя. Возможные значения:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ИМХО нужно не Права, а Право.
Ведь я правильно понимаю, что можно указать только одно единственное значение права? Из кода следует только этот вариант.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Исправил

УправлениеКонфигуратором.СоздатьФайловоеХранилищеКонфигурации(
КаталогВременногоХранилища,
"Администратор");
Утверждения.ПроверитьИстину(УправлениеКонфигуратором.ХранилищеКонфигурацииСуществует(КаталогВременногоХранилища), "Временное хранилище конфигурации должно существовать");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, нужно перенести этот метод из API

@khorevaa
Copy link
Member Author

Вроде все поправил. Выделение сделаю отдельным PR
Например, ДобавитьПараметрыПодключенияКХранилищу( Параметры, Знач СтрокаСоединения, Знач ПользовательХранилища, Знач ПарольХранилища)

@nixel2007 nixel2007 merged commit 12a34af into oscript-library:master Jan 19, 2017
@khorevaa khorevaa deleted the feature/ConfigurationRepositoryAddUser branch January 19, 2017 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants