Refactore create_project_package to fix Rubocop offence#876
Refactore create_project_package to fix Rubocop offence#876kostyanf14 wants to merge 1 commit intoHCK-CI:masterfrom
Conversation
Summary of ChangesHello @kostyanf14, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing code quality and maintainability by refactoring a key method responsible for creating project packages. The changes extract specific functionalities into dedicated methods, making the code more modular and easier to understand. This refactoring also incorporates improved logging for cases where specified file paths are invalid, contributing to more robust error handling. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The refactoring successfully addresses the Rubocop offense by breaking down the create_project_package method into smaller, more manageable pieces. However, the two new helper methods introduced (prepare_package_driver_content and prepare_package_supplemental_content) contain highly redundant logic. Consolidating this logic into a single private helper method would improve maintainability and readability. Additionally, since the project uses Sorbet for type checking, adding type signatures to these new methods would ensure consistency with the rest of the codebase.
lib/engines/hcktest/tests.rb
Outdated
| def prepare_package_driver_content | ||
| package_with_driver = @project.options.test.package_with_driver | ||
| driver_path = @project.options.test.driver_path | ||
|
|
||
| remote_driver_path = nil | ||
| remote_supplemental_path = nil | ||
|
|
||
| if package_with_driver && driver_path && File.exist?(driver_path) | ||
| remote_driver_path = "#{STUDIO_PACKAGE_ASSETS_PATH}\\Driver" | ||
| @logger.info("Uploading driver from '#{driver_path}' to '#{remote_driver_path}'") | ||
| @tools.upload_to_studio(driver_path, remote_driver_path) | ||
| if package_with_driver && driver_path | ||
| if File.exist?(driver_path) | ||
| remote_driver_path = "#{STUDIO_PACKAGE_ASSETS_PATH}\\Driver" | ||
| @logger.info("Uploading driver from '#{driver_path}' to '#{remote_driver_path}'") | ||
| @tools.upload_to_studio(driver_path, remote_driver_path) | ||
| else | ||
| @logger.warn("Driver path '#{driver_path}' specified, but does not exist, skipping packaging") | ||
| end | ||
| end | ||
|
|
||
| if supplemental_path && File.exist?(supplemental_path) | ||
| remote_supplemental_path = "#{STUDIO_PACKAGE_ASSETS_PATH}\\Supplemental" | ||
| @logger.info("Uploading supplemental from '#{supplemental_path}' to '#{remote_supplemental_path}'") | ||
| @tools.upload_to_studio(supplemental_path, remote_supplemental_path) | ||
| remote_driver_path | ||
| end | ||
|
|
||
| def prepare_package_supplemental_content | ||
| supplemental_path = @project.options.test.supplemental_path | ||
|
|
||
| remote_supplemental_path = nil | ||
| if supplemental_path | ||
| if File.exist?(supplemental_path) | ||
| remote_supplemental_path = "#{STUDIO_PACKAGE_ASSETS_PATH}\\Supplemental" | ||
| @logger.info("Uploading supplemental from '#{supplemental_path}' to '#{remote_supplemental_path}'") | ||
| @tools.upload_to_studio(supplemental_path, remote_supplemental_path) | ||
| else | ||
| @logger.warn("Supplemental path '#{supplemental_path}' specified, but does not exist, skipping packaging") | ||
| end | ||
| end | ||
|
|
||
| remote_supplemental_path | ||
| end |
There was a problem hiding this comment.
The methods prepare_package_driver_content and prepare_package_supplemental_content share almost identical logic for checking path existence, logging, and uploading assets. This duplication can be reduced by introducing a shared helper method. Furthermore, adding Sorbet type signatures would maintain consistency with other methods in this class.
sig { params(type: String, path: T.nilable(String)).returns(T.nilable(String)) }
def upload_package_asset(type, path)
return nil unless path
unless File.exist?(path)
@logger.warn("#{type} path '#{path}' specified, but does not exist, skipping packaging")
return nil
end
remote_path = "#{STUDIO_PACKAGE_ASSETS_PATH}\\\\#{type}"
@logger.info("Uploading #{type.downcase} from '#{path}' to '#{remote_path}'")
@tools.upload_to_studio(path, remote_path)
remote_path
end
sig { returns(T.nilable(String)) }
def prepare_package_driver_content
return unless @project.options.test.package_with_driver
upload_package_asset('Driver', @project.options.test.driver_path)
end
sig { returns(T.nilable(String)) }
def prepare_package_supplemental_content
upload_package_asset('Supplemental', @project.options.test.supplemental_path)
endThere was a problem hiding this comment.
Pull request overview
Refactors the HLK results packaging workflow in AutoHCK::Tests by extracting driver and supplemental upload logic into dedicated helper methods, addressing a RuboCop offense while keeping the packaging behavior intact.
Changes:
- Extracted driver upload logic into
prepare_package_driver_content. - Extracted supplemental upload logic into
prepare_package_supplemental_content. - Updated
create_project_packageto use the new helpers and to warn when configured paths don’t exist.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Kostiantyn Kostiuk <kkostiuk@redhat.com>
No description provided.