Skip to content

refactor: restructure workspace and migrate ros packages to .repos#25

Merged
jorgenfj merged 28 commits intomainfrom
refactor/filestructure
Apr 3, 2025
Merged

refactor: restructure workspace and migrate ros packages to .repos#25
jorgenfj merged 28 commits intomainfrom
refactor/filestructure

Conversation

@kluge7
Copy link
Copy Markdown
Contributor

@kluge7 kluge7 commented Mar 30, 2025

This PR introduces .repos file for managing ROS dependencies (instead of submodules) and apply formatting changes

@kluge7 kluge7 requested a review from Copilot March 30, 2025 16:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the file structure and cleans up YAML configuration files to improve readability and consistency.

  • Removed unnecessary dash markers and extraneous lines from various YAML files.
  • Updated pre-commit and CI workflow configurations for improved parameter formatting.
  • Adjusted the clang-format configuration for a cleaner style file.

Reviewed Changes

Copilot reviewed 10 out of 19 changed files in this pull request and generated no comments.

Show a summary per file
File Description
perception_setup/config/zed_driver_params.yaml Removed extraneous list markers to clean up configuration syntax.
perception_setup/config/pipeline_line_fitting_params.yaml Reformatted YAML structure by removing unnecessary separators.
perception_setup/config/blackfly_s_params.yaml Cleaned up YAML by eliminating redundant dash markers.
perception_setup/config/aruco_detector_params.yaml Simplified YAML structure with removed extraneous dashes.
.pre-commit-config.yaml Condensed arguments for hooks, improving readability.
.github/workflows/semantic-release.yml Minor cleanup by removing an extraneous line.
.github/workflows/industrial-ci.yml Added new parameters to support upstream dependency handling in CI.
.clang-format Removed an unnecessary separator to align with the intended style.
Files not reviewed (9)
  • .github/workflows/source-build.yaml: Language not supported
  • .gitmodules: Language not supported
  • dependencies.repos: Language not supported
  • scripts/ci_install_dependencies.sh: Language not supported
  • submodules/isaac_ros_common: Language not supported
  • submodules/vortex-aruco-detection: Language not supported
  • submodules/vortex-image-filtering: Language not supported
  • submodules/vortex-msgs: Language not supported
  • submodules/vortex-pipeline-detection: Language not supported

@kluge7 kluge7 changed the title refactor/filestructure refactor: restructure file layout, remove ros packages from submodules, and migrate to .repos Mar 30, 2025
@kluge7 kluge7 changed the title refactor: restructure file layout, remove ros packages from submodules, and migrate to .repos refactor: restructure workspace and migrate ros packages to .repos Mar 30, 2025
@kluge7 kluge7 requested a review from jorgenfj March 30, 2025 16:32
@jorgenfj
Copy link
Copy Markdown
Contributor

can remove vortex-vkf and vortex-msgs as dependencies for now as they are not used yet in this repository, only in vortex-auv.

@jorgenfj
Copy link
Copy Markdown
Contributor

Do we want to change vortex-blackfly-driver into a submodule?
This comes down to the question whether we want submodules to be sensor dependent or hardware(cuda) dependent

@kluge7 kluge7 marked this pull request as ready for review April 2, 2025 12:59
@kluge7 kluge7 requested a review from Copilot April 2, 2025 12:59
@kluge7 kluge7 self-assigned this Apr 2, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request restructures the workspace by migrating ROS dependencies from submodules to a .repos file and applies several formatting improvements across configuration and CI files.

  • Refactors YAML configuration files by removing redundant dash lines.
  • Updates pre-commit configuration with simplified argument arrays and adds a YAML formatting hook.
  • Adjusts GitHub workflow files with new parameters for CI and removes an extraneous header from .clang-format.

Reviewed Changes

Copilot reviewed 12 out of 21 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
perception_setup/config/zed_driver_params.yaml Removal of extraneous YAML list indicators to clean up formatting.
perception_setup/config/pipeline_line_fitting_params.yaml Minor formatting changes by removing redundant list separators.
perception_setup/config/blackfly_s_params.yaml Removed unnecessary dash lines for cleaner YAML structure.
perception_setup/config/aruco_detector_params.yaml Removed extra dash lines to maintain consistent YAML formatting.
README.md Added a note about required repositories listed in the dependencies.repos file.
.pre-commit-config.yaml Updated formatting of hook arguments and added a new YAML formatting hook from yamlfmt.
.github/workflows/semantic-release.yml Small formatting adjustments.
.github/workflows/industrial-ci.yml Added inputs for upstream workspace and ros_repo, modifying the workflow parameters.
.clang-format Removed an extraneous header line to match the intended style configuration.
Files not reviewed (9)
  • .github/workflows/source-build.yaml: Language not supported
  • .gitmodules: Language not supported
  • dependencies.repos: Language not supported
  • submodules/flir_camera_driver: Language not supported
  • submodules/isaac_ros_common: Language not supported
  • submodules/vortex-aruco-detection: Language not supported
  • submodules/vortex-image-filtering: Language not supported
  • submodules/vortex-msgs: Language not supported
  • submodules/vortex-pipeline-detection: Language not supported

uses: vortexntnu/vortex-ci/.github/workflows/reusable-industrial-ci.yml@main
with:
upstream_workspace: './dependencies.repos'
ros_repo: '["testing", "main"]'
Copy link

Copilot AI Apr 2, 2025

Choose a reason for hiding this comment

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

The 'ros_repo' parameter is provided as a stringified JSON array, which may not be interpreted correctly if a YAML list is expected. Consider converting it to a YAML list, for example: ros_repo: ["testing", "main"].

Suggested change
ros_repo: '["testing", "main"]'
ros_repo:
- testing
- main

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor

@jorgenfj jorgenfj left a comment

Choose a reason for hiding this comment

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

lgtm

@jorgenfj jorgenfj merged commit 575cd1e into main Apr 3, 2025
3 checks passed
@jorgenfj jorgenfj deleted the refactor/filestructure branch April 3, 2025 14:34
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 3, 2025

🎉 This PR is included in version 1.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants