Доработки gitsync tags push tempdir authors branch#6
Hidden character warning
Доработки gitsync tags push tempdir authors branch#6khorevaa wants to merge 30 commits intooscript-library:masterfrom khorevaa:Feature/Доработки-gitsync-tags-push-tempdir-authors
Conversation
…ый-n-версий-хранилища to master * commit '1a0a08b61b201b763fb81a89b23683bce0084ff0': Исправлен вызов процедуры синхронизации с новыми параметрами Добавление отправки на удаленный репо каждые n - коммитов
…параметра-ветки-в-gitsync-all # Conflicts: # src/multi-controller.os # src/xml-config.os
…-ветки-в-gitsync-all to master * commit 'e9aca5e974c44d326b7bb178b5d853faea05c290': Добавлена возможность указания ветки при пакетной синхронизации
…и-указания-расположения-рабочего-каталога to develop * commit '7d55345447bf90ae41ebbefb239cfdb4ed835fca': Исправлена ошибка преобразования типов Дорабавлен ключ "-tempcatalog" Добавлена возможность указания Temp каталога
* commit 'f2decf724a37db26a262ea684a4d941b3d9c7e72': Исправлена ошибка преобразования типов Дорабавлен ключ "-tempcatalog" Добавлена возможность указания Temp каталога
* commit 'b2e6edb60af6762b933d33ac92ef5e3150a181c9':
# Conflicts: # src/core/Классы/МенеджерСинхронизации.os
Добавлены консольные команды
* commit '706464c9de369447b4992c3688002eee7178c923': Очередная правка багов Исправлены баги. Добавлены консольные команды Переработка хранения файла AUTHORS Добавил Экспорт методу
* commit 'b260cc968ac0f1c587dc0d91cc1101859091c121': Очередная правка багов Исправлены баги. Добавлены консольные команды Переработка хранения файла AUTHORS Добавил Экспорт методу
* commit '9af89bfb6f043581cc6cd2ecc6a3d8ef50df32a3':
| @@ -0,0 +1,3 @@ | |||
| // Поместите параметры в этот файл, чтобы перезаписать параметры по умолчанию и пользовательские параметры. | |||
There was a problem hiding this comment.
Этот файл разве нужен в гите? Пустой по крайней мере.
There was a problem hiding this comment.
Да не корректный gitignore...
There was a problem hiding this comment.
Эти переменные точно экспортные? Обычно через м все же приватные добавляют
There was a problem hiding this comment.
@nixel2007 не к той строчке прикрепил. Это экспортные. Это свойства объекта. Их добавлял я. А вот те, что ниже - да, твой вопрос актуален.
There was a problem hiding this comment.
Хм... подумаю... видимо для консольной команды надо было..
There was a problem hiding this comment.
Новые параметры без дефолтных значаний
There was a problem hiding this comment.
Дополню. Зачем 2 параметра? Установить метку и сама метка. Почему не сделать только "Метка" и если она указана, то устанавливать ее?
There was a problem hiding this comment.
Да верно... Не подумал. Исправлю! . :)
There was a problem hiding this comment.
Согласен, нужно перенести в отладочный лог.
There was a problem hiding this comment.
Такое чувство, что перенесли местами процедуры, но как-то не аккуратно
There was a problem hiding this comment.
Ага, начиная с этого места, начинаются проблемы с код-ревью.
Гит просто показывается совершенно разные строки.
@khorevaa Предлагаю вернуть порядок функций, иначе сейчас очень-очень трудно выполнить полноценный код-ревью.
There was a problem hiding this comment.
Порядок функций я не менял... просто почистил лишние и неиспользуемые..... как то так.. что с этим делать не знаю... вернуть старые функции?
There was a problem hiding this comment.
@khorevaa нет, просто сделать изменения более мелкими.
There was a problem hiding this comment.
Да все верно ковертация теперь не делается...
There was a problem hiding this comment.
См. выше. 2 параметра не нужны.
Если ЗначениеЗаполнено(Метка) Тогда
// устанавливаем метку
There was a problem hiding this comment.
Ага, начиная с этого места, начинаются проблемы с код-ревью.
Гит просто показывается совершенно разные строки.
@khorevaa Предлагаю вернуть порядок функций, иначе сейчас очень-очень трудно выполнить полноценный код-ревью.
There was a problem hiding this comment.
Зачем вообще сей функционал? Какой сценарий использования?
Напомню: мы пытались добавить функционал веток и поняли, что он не нужен.
There was a problem hiding this comment.
Функционал нужен для корректной отправки tags. Да и имя бранча мне нужно было т.к. выгружалось девелопорское хранилище.. (не грузить же его в мастер) .....
There was a problem hiding this comment.
@khorevaa у вас всегда пуш/пулл делается в текущую ветку. Вот какую установили текущей, такая и синхронизируется. Явное указание ветки внутри gitsync потребует еще и команду checkout. А ее нет.
Т.е. сейчас вы например на ветке develop. Сделали синк с параметром ветки мастер, оно разложило, сделало пуш в мастер (но изменений для мастера нет, они выгрузились и закоммитились в текущий develop). В результате пуш будет пустой.
There was a problem hiding this comment.
@khorevaa подскажи, для чего эта доработка?
Такой масштабный PR, я бы, если честно, порезал его на более мелкие, 1 функциональность на один пулреквест.
There was a problem hiding this comment.
+1 за более мелкие PR.
С другой стороны, автору ИМХО будет достаточно тяжело выделить мелкие изменения :(
There was a problem hiding this comment.
Да с выделением будет много проблем.. Но в принципе можно....
There was a problem hiding this comment.
Да и решал я коллизии...
There was a problem hiding this comment.
Зачем удален код по установке базового формата имени автора для гит?
это код нужен для того, если в файле авторов указан неверный формат данных или вообще не задан файл авторов.
There was a problem hiding this comment.
Там еще и AUTHORS в json превращен, насколько я понял.
|
Я против вливания данного PR. Часть возможностей хорошая и правильная, часть весьма спорная. Одним мегакуском это разруливать сложно. Тут аж 6 функциональных возможностей предлагается. Это, в моем понимании, должно быть 6 реквестов. |
|
Да, я также склоняюсь к мнению, что данный PR заливать рановато. |
@EvilBeaver Наше обсуждение в гиттере https://gitter.im/EvilBeaver/oscript-library?at=582ee36699dce7860aff3244 (смотреть чуть выше и ниже по тексту) |
|
@artbear в гиттер идет обсуждение пушей через n коммитов. Отличная доработка, никто не против. |
|
Буду дробить функциональность на несколько PR... |
Полный рефакторинг приложения и библиотеки
4 participants
Добавлены следующие возможности: