Skip to content

feat: add support for fish#1211

Open
Sunrisepeak wants to merge 1 commit intoros2:rollingfrom
Sunrisepeak:feat/add_support_for_fish
Open

feat: add support for fish#1211
Sunrisepeak wants to merge 1 commit intoros2:rollingfrom
Sunrisepeak:feat/add_support_for_fish

Conversation

@Sunrisepeak
Copy link

Description

add support for fish

Is this user-facing behavior change?

improve for fish shell user

Did you use Generative AI?

Additional Information

- ament/ament_package#164
- ros2#326

Signed-off-by: sunrisepeak <speakshen@163.com>
Copy link
Member

@skyegalaxy skyegalaxy left a comment

Choose a reason for hiding this comment

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

(originally had meant to leave the comment here instead)

@Sunrisepeak - I ran your branch locally and the generation of package level fish scripts seems to be working well! I'm a bit confused about your testing setup, however. I would expect that by switching my rolling workspace's ament_package to this branch, that I would see a top level install/setup.fish inside my workspace's install folder, but that doesn't seem to be the case and your test script seems to manually add the setup.fish to /opt/ros/$DISTRO. Is adding the top level autogenerated setup.fish out of scope for this PR?

@Sunrisepeak
Copy link
Author

(originally had meant to leave the comment here instead)

@Sunrisepeak - I ran your branch locally and the generation of package level fish scripts seems to be working well! I'm a bit confused about your testing setup, however. I would expect that by switching my rolling workspace's ament_package to this branch, that I would see a top level install/setup.fish inside my workspace's install folder, but that doesn't seem to be the case and your test script seems to manually add the setup.fish to /opt/ros/$DISTRO. Is adding the top level autogenerated setup.fish out of scope for this PR?

@skyegalaxy ament/ament_package#164 (comment)

@skyegalaxy
Copy link
Member

Pulls: ament/ament_package#164, #1211
Gist: https://gist.githubusercontent.com/skyegalaxy/4ab9923e4f12b167a21f2a368ffab9b3/raw/fa2526eb1ea4695d8c8a5bd2d39a0709237d25cb/fish_support.repos
BUILD args: --packages-above-and-dependencies ament_package
TEST args: --packages-above ament_package
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/18569/

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@Sunrisepeak
Copy link
Author

@skyegalaxy the ci-unstable seem not related with the pr, Does it need to be rerun?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants