-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fixed bug with looping #68
Conversation
WalkthroughThe recent modifications across multiple modules primarily involve renaming and redirecting function calls from Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
This comment has been minimized.
This comment has been minimized.
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.
Actionable comments posted: 6
Out of diff range and nitpick comments (1)
src/cf/CommonModules/__ОбщегоНазначенияСервер/Ext/Module.bsl (1)
Line range hint
813-833
: Ensure the functionОбщийМодуль
handles the case where the module does not exist more gracefully.Функция ОбщийМодуль(Имя) Экспорт Если Метаданные.ОбщиеМодули.Найти(Имя) <> Неопределено Тогда УстановитьБезопасныйРежим(Истина); Модуль = Вычислить(Имя); ИначеЕсли СтрЧислоВхождений(Имя, ".") = 1 Тогда Возврат СерверныйМодульМенеджера(Имя); Иначе Модуль = Неопределено; КонецЕсли; Если ТипЗнч(Модуль) <> Тип("ОбщийМодуль") Тогда ВызватьИсключение __СтроковыеФункцииСлужебныйКлиентСервер.ПодставитьПараметрыВСтроку( НСтр("ru = 'Общий модуль ""%1"" не существует.'"), Имя); КонецЕсли; Возврат Модуль; КонецФункцииThis function attempts to fetch a module by name but does not handle the case where the module does not exist in a user-friendly manner. Consider adding a more descriptive error message or handling this case differently to avoid runtime errors.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- src/cf/CommonModules/__ЖурналРегистрацииСлужебный/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__ОбщегоНазначенияВызовСервера/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__ОбщегоНазначенияКлиент/Ext/Module.bsl (2 hunks)
- src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__ОбщегоНазначенияСервер/Ext/Module.bsl (3 hunks)
- src/cf/CommonModules/__ОбщегоНазначенияСлужебный/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__ОбщегоНазначенияСлужебныйКлиентСервер/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__ПолучениеФайловИзИнтернетаСлужебный/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__ПользователиСлужебный/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__РегламентныеЗаданияСлужебныйСервер/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__СтандартныеПодсистемыСлужебныйПовтИсп/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__СтроковыеФункцииСлужебныйКлиентСервер/Ext/Module.bsl (1 hunks)
Additional Context Used
GitHub Check Runs (1)
1C: Project SubSystems Library Sonarqube Results success (5)
src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl: [warning] 109-109: src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl#L109
Добавьте описание возвращаемого значения функции
src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl: [warning] 109-109: src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl#L109
Необходимо добавить описание всех параметров метода
src/cf/CommonModules/__ПолучениеФайловИзИнтернетаСлужебный/Ext/Module.bsl: [warning] 40-40: src/cf/CommonModules/__ПолучениеФайловИзИнтернетаСлужебный/Ext/Module.bsl#L40
Длина строки 123 превышает максимально допустимую 120
src/cf/CommonModules/__РегламентныеЗаданияСлужебныйСервер/Ext/Module.bsl: [warning] 56-56: src/cf/CommonModules/__РегламентныеЗаданияСлужебныйСервер/Ext/Module.bsl#L56
Длина строки 121 превышает максимально допустимую 120
src/cf/CommonModules/__СтандартныеПодсистемыСлужебныйПовтИсп/Ext/Module.bsl: [warning] 40-40: src/cf/CommonModules/__СтандартныеПодсистемыСлужебныйПовтИсп/Ext/Module.bsl#L40
Длина строки 127 превышает максимально допустимую 120
Additional comments not posted (5)
src/cf/CommonModules/__ПользователиСлужебный/Ext/Module.bsl (1)
40-40
: Change aligns with PR objectives and appears correctly implemented.src/cf/CommonModules/__ЖурналРегистрацииСлужебный/Ext/Module.bsl (1)
41-41
: Change aligns with PR objectives and appears correctly implemented.src/cf/CommonModules/__ОбщегоНазначенияВызовСервера/Ext/Module.bsl (1)
31-33
: Change aligns with PR objectives and appears correctly implemented.src/cf/CommonModules/__СтроковыеФункцииСлужебныйКлиентСервер/Ext/Module.bsl (1)
63-63
: Change aligns with PR objectives and appears correctly implemented.src/cf/CommonModules/__ОбщегоНазначенияСлужебныйКлиентСервер/Ext/Module.bsl (1)
155-155
: Change aligns with PR objectives and appears correctly implemented.
// Возвращает ссылку на общий модуль или модуль менеджера по имени. Если подключена БСП, | ||
// происходит переадресация на общий модуль из БСП, иначе используются встроенные модули ПБП. | ||
// | ||
// Параметры: | ||
// ИмяМодуляБСП - Строка - Имя Общего модуля из Библиотеки стандартных подсистем | ||
// ИмяМодуляВстроенного - Строка - Имя Общего модуля из Проектной библиотеки подсистем | ||
// | ||
// Возвращаемое значение: | ||
// ОбщийМодуль | ||
// МодульМенеджераОбъекта | ||
// | ||
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного = "") Экспорт | ||
|
||
Имя = ИмяМодуляВстроенного; | ||
Если __ОбщегоНазначенияПовтИсп.СуществуетБиблиотекаСтандартныхПодсистем() | ||
И Метаданные.ОбщиеМодули.Найти(ИмяМодуляБСП) <> Неопределено Тогда | ||
Имя = ИмяМодуляБСП; | ||
КонецЕсли; | ||
|
||
Модуль = ОбщийМодуль(Имя); | ||
|
||
Возврат Модуль; | ||
|
||
КонецФункции |
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.
Refactor the function ПереадресацияОбщегоМодуля
to improve readability and maintainability.
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного = "") Экспорт
Имя = ИмяМодуляВстроенного;
Если __ОбщегоНазначенияПовтИсп.СуществуетБиблиотекаСтандартныхПодсистем() И Метаданные.ОбщиеМодули.Найти(ИмяМодуляБСП) <> Неопределено Тогда
Имя = ИмяМодуляБСП;
КонецЕсли;
Возврат ОбщийМодуль(Имя);
КонецФункции
This refactoring simplifies the function by removing unnecessary blank lines and aligning the control flow structures for better readability.
@@ -155,7 +155,7 @@ | |||
#Область СлужебныеПроцедурыИФункции | |||
|
|||
Функция ПолучитьМодуль() | |||
Возврат __ОбщегоНазначенияПовтИсп.ОбщийМодуль("ОбщегоНазначения", "__ОбщегоНазначенияСервер"); | |||
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("ОбщегоНазначения", "__ОбщегоНазначенияСервер"); |
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.
Tip
Codebase Verification
The verification process has revealed that the old function ОбщийМодуль
is still being used extensively across various modules in the codebase. This contradicts the PR's objective to replace it with ПереадресацияОбщегоМодуля
to address looping issues. The occurrences are not isolated to a specific area but are widespread, indicating that the replacement has not been fully implemented.
Locations still using ОбщийМодуль
:
src/cfe/YAXUnit/DataProcessors/ЮТКонструкторТестовыхДанных/Forms/КлиентскийКонструктор/Ext/Form/Module.bsl
src/cfe/YAXUnit/CommonModules/ЮТест/Ext/Module.bsl
src/cfe/YAXUnit/CommonModules/ЮТУтвержденияИБ/Ext/Module.bsl
src/cf/CommonModules/__ОбщегоНазначенияСервер/Ext/Module.bsl
src/cf/CommonModules/__ОбщегоНазначенияКлиент/Ext/Module.bsl
src/cf/Catalogs/__ПользовательскиеФункции/Forms/ФормаЭлемента/Ext/Form/Module.bsl
- Many more locations as listed in the output.
These findings suggest that the transition to the new function is incomplete, and further work is needed to ensure consistency across the entire codebase.
Analysis chain
Change aligns with PR objectives to address looping issues by updating module access method.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that `ПереадресацияОбщегоМодуля` is consistently used across the codebase.
# Test: Search for the old function usage. Expect: No occurrences of the old function.
rg --type bsl $'ОбщийМодуль'
Length of output: 135
Script:
#!/bin/bash
# Description: Verify that `ПереадресацияОбщегоМодуля` is consistently used across the codebase and `ОбщийМодуль` is not used.
# Test: Search for the old function usage across all files, as the file type 'bsl' was not recognized previously.
rg 'ОбщийМодуль' --text
Length of output: 19964
// См. __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля. | ||
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного) Экспорт | ||
|
||
Возврат __ОбщегоНазначенияСервер.ОбщийМодуль(ИмяМодуляБСП, ИмяМодуляВстроенного); | ||
Возврат __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного); |
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.
Function ПереадресацияОбщегоМодуля
correctly replaces ОбщийМодуль
. Add descriptions for parameters and return value for clarity.
+ // Параметры:
+ // ИмяМодуляБСП - Строка - имя базового модуля.
+ // ИмяМодуляВстроенного - Строка - имя встроенного модуля.
+ //
+ // Возвращаемое значение:
+ // ОбщийМодуль - ссылка на общий модуль.
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
// См. __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля. | |
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного) Экспорт | |
Возврат __ОбщегоНазначенияСервер.ОбщийМодуль(ИмяМодуляБСП, ИмяМодуляВстроенного); | |
Возврат __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного); | |
// См. __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля. | |
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного) Экспорт | |
// Параметры: | |
// ИмяМодуляБСП - Строка - имя базового модуля. | |
// ИмяМодуляВстроенного - Строка - имя встроенного модуля. | |
// | |
// Возвращаемое значение: | |
// ОбщийМодуль - ссылка на общий модуль. | |
Возврат __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного); |
@@ -37,7 +37,7 @@ | |||
#Область СлужебныеПроцедурыИФункции | |||
|
|||
Функция ПолучитьМодуль() | |||
Возврат __ОбщегоНазначенияПовтИсп.ОбщийМодуль("СтандартныеПодсистемыПовтИсп", "__СтандартныеПодсистемыПовтИсп"); | |||
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("СтандартныеПодсистемыПовтИсп", "__СтандартныеПодсистемыПовтИсп"); |
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.
Change aligns with PR objectives but consider breaking the line to adhere to the maximum line length of 120 characters.
- Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("СтандартныеПодсистемыПовтИсп", "__СтандартныеПодсистемыПовтИсп");
+ Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля(
+ "СтандартныеПодсистемыПовтИсп", "__СтандартныеПодсистемыПовтИсп");
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("СтандартныеПодсистемыПовтИсп", "__СтандартныеПодсистемыПовтИсп"); | |
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля( | |
"СтандартныеПодсистемыПовтИсп", "__СтандартныеПодсистемыПовтИсп"); |
@@ -37,7 +37,7 @@ | |||
#Область СлужебныеПроцедурыИФункции | |||
|
|||
Функция ПолучитьМодуль() | |||
Возврат __ОбщегоНазначенияПовтИсп.ОбщийМодуль("ПолучениеФайловИзИнтернета", "__ПолучениеФайловИзИнтернета"); | |||
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("ПолучениеФайловИзИнтернета", "__ПолучениеФайловИзИнтернета"); |
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.
Change aligns with PR objectives but consider breaking the line to adhere to the maximum line length of 120 characters.
- Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("ПолучениеФайловИзИнтернета", "__ПолучениеФайловИзИнтернета");
+ Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля(
+ "ПолучениеФайловИзИнтернета", "__ПолучениеФайловИзИнтернета");
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("ПолучениеФайловИзИнтернета", "__ПолучениеФайловИзИнтернета"); | |
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля( | |
"ПолучениеФайловИзИнтернета", "__ПолучениеФайловИзИнтернета"); |
@@ -53,7 +53,7 @@ | |||
#Область СлужебныеПроцедурыИФункции | |||
|
|||
Функция ПолучитьМодуль() | |||
Возврат __ОбщегоНазначенияПовтИсп.ОбщийМодуль("РегламентныеЗаданияСервер", "__РегламентныеЗаданияСервер"); | |||
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("РегламентныеЗаданияСервер", "__РегламентныеЗаданияСервер"); |
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.
Change aligns with PR objectives but consider breaking the line to adhere to the maximum line length of 120 characters.
- Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("РегламентныеЗаданияСервер", "__РегламентныеЗаданияСервер");
+ Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля(
+ "РегламентныеЗаданияСервер", "__РегламентныеЗаданияСервер");
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля("РегламентныеЗаданияСервер", "__РегламентныеЗаданияСервер"); | |
Возврат __ОбщегоНазначенияПовтИсп.ПереадресацияОбщегоМодуля( | |
"РегламентныеЗаданияСервер", "__РегламентныеЗаданияСервер"); |
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.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__ПолучениеФайловИзИнтернетаСлужебный/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__РегламентныеЗаданияСлужебныйСервер/Ext/Module.bsl (1 hunks)
- src/cf/CommonModules/__СтандартныеПодсистемыСлужебныйПовтИсп/Ext/Module.bsl (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/cf/CommonModules/__ПолучениеФайловИзИнтернетаСлужебный/Ext/Module.bsl
- src/cf/CommonModules/__РегламентныеЗаданияСлужебныйСервер/Ext/Module.bsl
- src/cf/CommonModules/__СтандартныеПодсистемыСлужебныйПовтИсп/Ext/Module.bsl
Additional Context Used
GitHub Check Runs (1)
1C: Project SubSystems Library Sonarqube Results success (2)
src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl: [warning] 110-110: src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl#L110
Добавьте описание возвращаемого значения функции
src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl: [warning] 110-110: src/cf/CommonModules/__ОбщегоНазначенияПовтИсп/Ext/Module.bsl#L110
Необходимо добавить описание всех параметров метода
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного) Экспорт | ||
|
||
Возврат __ОбщегоНазначенияСервер.ОбщийМодуль(ИмяМодуляБСП, ИмяМодуляВстроенного); | ||
Возврат __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного); |
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.
Add descriptions for parameters and return value to enhance clarity and maintainability.
+ // Параметры:
+ // ИмяМодуляБСП - Строка - имя базового модуля.
+ // ИмяМодуляВстроенного - Строка - имя встроенного модуля.
+ //
+ // Возвращаемое значение:
+ // ОбщийМодуль - ссылка на общий модуль.
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного) Экспорт
Возврат __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного);
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного) Экспорт | |
Возврат __ОбщегоНазначенияСервер.ОбщийМодуль(ИмяМодуляБСП, ИмяМодуляВстроенного); | |
Возврат __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного); | |
// Параметры: | |
// ИмяМодуляБСП - Строка - имя базового модуля. | |
// ИмяМодуляВстроенного - Строка - имя встроенного модуля. | |
// | |
// Возвращаемое значение: | |
// ОбщийМодуль - ссылка на общий модуль. | |
Функция ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного) Экспорт | |
Возврат __ОбщегоНазначенияСервер.ПереадресацияОбщегоМодуля(ИмяМодуляБСП, ИмяМодуляВстроенного); |
Summary by CodeRabbit
Refactor
Bug Fixes
Documentation