-
Notifications
You must be signed in to change notification settings - Fork 53
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.
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
Переход на EDT 2023.3 #1386
Переход на EDT 2023.3 #1386
Conversation
…ески размещать его в соответствующей области
G5V8DT-24027. Сортировка объектов и прав в ролях
…ески размещать его в соответствующей области Copyright text change and style changes
…ески размещать его в соответствующей области Minor changes
…ески размещать его в соответствующей области
…ески размещать его в соответствующей области Rename extension point Versioning
…ь его в соответствующей области Another refactoring
…ески размещать его в соответствующей области
Поднятие версий com._1c.g5.v8.dt.bsl.validation
…ески размещать его в соответствующей области Code style fixes
G5V8DT-24189 При добавлении обработчика события или команды автоматически размещать его в соответствующей области
RedundantExportMethodCheckTest.testCallNoPublic
Feature/1378: Добавлены проверки на null
….validation.marker Поднятие версий dt.validation.marker
|
@@ -68,6 +69,7 @@ public void testNoCallPublic() throws Exception | |||
} | |||
|
|||
@Test | |||
@Ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно указывать задачу которая пофиксит проблему и включит тест
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Баг есть в джире. Проблема в EDT, не в codestyle - G5V8DT-24472
@@ -66,6 +67,7 @@ protected String getModuleFileName() | |||
* @throws Exception the exception | |||
*/ | |||
@Test | |||
@Ignore("https://github.com/1C-Company/v8-code-style/issues/1376") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нужно включить тест, т.к. пофиксили сам тест - при чем есть сомнения что исправление правильное! См. тут комменты к PR #1380
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vadimeg посмотри, пожалуйста
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Если исправление верное и тест ходит, то игнор можно убрать. Если нет, то нужно переделывать.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ответил по изменениям в парсинге ссылок тут #1380 (comment)
Тест надо включать.
@@ -58,6 +59,7 @@ public void testDataLock() throws Exception | |||
} | |||
|
|||
@Test | |||
@Ignore("https://github.com/1C-Company/v8-code-style/issues/1375") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Что с этим делать? Не ясно, пофиксили это в коре ЕДТ или нужно приделать проверку.
@@ -62,6 +63,7 @@ public void testFunctionHasDescriptionSection() throws Exception | |||
* @throws Exception the exception | |||
*/ | |||
@Test | |||
@Ignore("https://github.com/1C-Company/v8-code-style/issues/1377") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Что с этим тестом? Если тест падает - поменялось поведение. Что дальше? Где ошибка: в ЕДТ или тест переделывать?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не понимаю, что ты хочешь, ошибку на исправление завели, когда до нее дойдем, тогда и поймем, где не так
@@ -73,6 +74,7 @@ public void testCommonModule() throws Exception | |||
* @throws Exception | |||
*/ | |||
@Test | |||
@Ignore("https://github.com/1C-Company/v8-code-style/issues/1377") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Что с этим тестом? Если тест падает - поменялось поведение. Что дальше? Где ошибка: в ЕДТ или тест переделывать?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не понимаю, что ты хочешь, ошибку на исправление завели, когда до нее дойдем, тогда и поймем, где не так
case TABLE: | ||
return ModuleStructureSection.FORM_TABLE_ITEMS_EVENT_HANDLERS.getName(scriptVariant); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это не правильный регион - он должен включать Суффикс имени таблицы.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
так он потом добавиться суффикс
String suffix = getSuffix(eventOwner, itemType);
в com.e1c.v8codestyle.internal.bsl.ui.services.BslModuleRegionsInfoService.getEventHandlerTextInsertInfo(IXtextDocument, Module, int, IBslModuleEventData)
обработчику формы
обработчику формы правка тестов
G5V8DT-24674 Взводится признак наличия изменений при переходе к обработчику формы
followed by lines of code
followed by lines of code Правки по ревью
G5V8DT-24086 Ложные срабатывания проверки The asynchronous method is followed by lines of code
ModuleStructureSection[] declaredRegionNames = ModuleStructureSection.values(); | ||
Map<String, BslModuleOffsets> regionOffsets = new HashMap<>(declaredRegionNames.length / 2); | ||
for (RegionPreprocessor regionPreprocessor : regionPreprocessors) | ||
{ | ||
INode nodeAfter = NodeModelUtils.findActualNodeFor(regionPreprocessor.getItemAfter()); | ||
if (nodeAfter != null) | ||
{ | ||
String preprocessorRegionName = regionPreprocessor.getName(); | ||
for (int regionNameIndex = 0; regionNameIndex < declaredRegionNames.length; regionNameIndex++) | ||
{ | ||
ModuleStructureSection moduleStructureSection = declaredRegionNames[regionNameIndex]; | ||
String declaredRegionName = moduleStructureSection.getName(scriptVariant); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не правильно использовать порядок перечисление из базовых значений ModuleStructureSection
.
В каждом типе модулей свой порядок областей - нужно брать из com.e1c.v8codestyle.bsl.ModuleStructure.FORM_MODULE
например.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kudesunik Никит, а разве мы тут что-то создаем, какие-то области, мы просто находим существующие, но вот, когда мы их порождаем, мы следуем тому, что говорит Дима, порождаем их в правильном порядке?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Да, создаем (не конкретно здесь, а в getNewRegionOffset
). Да, в правильном порядке. ModuleStructureSection
отсортирован и сейчас по общему порядку полностью совпадает с ModuleStructure
.
Я основывался на документе структуры модулей и не уверен, что видел ModuleStructure
, но сейчас сходу не понятно, что делать, например, если в модуле окажется какой-нибудь несуществующий в ModuleStructure
DEPRECATED_REGION
или какие-нибудь другие регионы. Сейчас у меня он учтется и все регионы создадутся сверху него. Так что вопрос только в том, что если в одном модуле будет один порядок, а в другом модуле - другой порядок, не совпадающие между собой и соответственно с ModuleStructureSection
, то это сломается. Если такое планируется, то да, нужно переводить всю логику на частный порядок, определять что это за модуль и создавать регионы только на основе того, что прописано в ModuleStructure
. Если общий порядок и логика валидна, то и смысла нет.
|
Что сделано
Чек-лист
Общее:
master
и нет конфликтовЕсли применимо:
Закрываемые задачи
Closes #??