Skip to content
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

Skip rosdep install #829

Closed
kaichie opened this issue Jul 29, 2023 · 8 comments · Fixed by #830
Closed

Skip rosdep install #829

kaichie opened this issue Jul 29, 2023 · 8 comments · Fixed by #830
Labels
enhancement New feature or request

Comments

@kaichie
Copy link
Contributor

kaichie commented Jul 29, 2023

Description

Option to skip rosdep install. Useful for project that use a specific version of image for CI and in production.

Completion Criteria

  • Skip rosdep install in the action

Implementation Notes / Suggestions

  • Add an optional flag to skip rosdep install here?
@kaichie kaichie added the enhancement New feature or request label Jul 29, 2023
@christophebedard
Copy link
Member

christophebedard commented Jul 29, 2023

If dependencies are already installed, then rosdep should be pretty quick and will simply install nothing. Nonetheless, a pull request adding this feature would be welcome!

@kaichie
Copy link
Contributor Author

kaichie commented Jul 30, 2023

@christophebedard I have created the PR, thanks for getting back so quickly!

It will be mainly used to fail the build if the core dependencies are not in the image. However, it will not fail for the missing <exec_depend> dependencies. What do you think of adding another step to check the missing dependencies when the action is used with skip-rosdep-install?

@christophebedard
Copy link
Member

Tests should fail if <exec_depend> dependencies are missing, no?

@kaichie
Copy link
Contributor Author

kaichie commented Aug 6, 2023

Tests should fail if <exec_depend> dependencies are missing, no?

True if the package unit tests are covering the component. Just thinking for cases where test coverage are minimum(only testing for linting/formatting).

@christophebedard
Copy link
Member

That isn't ideal, but that's true. I think it would depend on what that <exec_depend> dependencies check would look like then. If it's simple enough, then we could consider adding it (optionally).

@kaichie
Copy link
Contributor Author

kaichie commented Aug 14, 2023

Added the dependencies check using rosdep check as an optional step when used with skip-rosdep-install. Please let me know if it sounds good.

@roncapat
Copy link

roncapat commented Dec 4, 2023

Any updates on this?

@christophebedard
Copy link
Member

The PR is here: #830. I just reviewed it, sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants