Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Develop #74

Closed
wants to merge 2 commits into from
Closed

Develop #74

wants to merge 2 commits into from

Conversation

Berckk
Copy link
Contributor

@Berckk Berckk commented Apr 16, 2018

Добавил методы по работе с хранилищем расширения конфигурации.
Добавил тесты для версии платформы 8.3.12.
Прошу проверить и принять.

EvilBeaver
EvilBeaver reviewed Apr 16, 2018
View reviewed changes
src/v8runner.os

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

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

Choose a reason for hiding this comment

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

Если я не ошибаюсь, в целях сокращения количества параметров был введен метод устанавливающий контекст соединения с хранилищем. Т.е. по аналогии с глобальной переменной подключения к базе вводили переменную, хранящую параметры подключения к хранилищу. @artbear @nixel2007 я ничего не путаю?

Copy link
Member

Choose a reason for hiding this comment

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

@khorevaa

Copy link
Member

Choose a reason for hiding this comment

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

@Berckk https://github.com/khorevaa/v8storage

Copy link
Contributor Author

@Berckk Berckk Apr 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Не видел v8storage - добавил бы туда. И ссылок не нашел поэтому понадобовлял здесь методов. Создал oscript-library/v8storage#4

Copy link
Member

Choose a reason for hiding this comment

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

@Berckk @khorevaa а здесь тогда закрываю, верно?

Copy link
Contributor 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.

Думаю да..
@Berckk сделаешь PR в v8storage?

@artbear
Copy link
Member

artbear commented Apr 16, 2018

Хорошая работа проделана.
Только вопрос - может быть, все-таки перенести в v8storage от @khorevaa , а не в v8runner ?

@khorevaa
Copy link
Member

khorevaa commented Apr 17, 2018 •
edited
Loading

Работа отличная -)) Думаю стоит в том числе перенести v8storage.

@EvilBeaver EvilBeaver closed this Jun 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@khorevaa khorevaa khorevaa left review comments

@nixel2007 nixel2007 nixel2007 left review comments

@EvilBeaver EvilBeaver EvilBeaver left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Berckk @artbear @khorevaa @nixel2007 @EvilBeaver