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

Feature/support conan package creation from local build #32

Conversation

maxfaine
Copy link
Contributor

Feature/support conan package creation from local build

Copy link
Contributor

@gregmedd gregmedd left a comment

Choose a reason for hiding this comment

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

This change makes it impossible to build this library on a standard Ubuntu 22.04 machine. I would recommend testing using the instructions in README.md in a clean Ubuntu environment (e.g. docker run -it ubuntu:22.04) to verify your changes are good. Feel free to update the README if there are any process changes (e.g. build steps, flags for conan or cmake, etc).

The process of verifying the build steps will start with building and installing zenoh-c.

I'd also like for you to update both the commits and this review with a meaningful description of what these changes are and what they do. The commits can be squashed and reworded however you best feel conveys the content of your work using git rebase -i ....

conanfile.py Show resolved Hide resolved
conanfile.py Show resolved Hide resolved
conanfile.py Show resolved Hide resolved
conanfile.py Show resolved Hide resolved
lib/CMakeLists.txt Show resolved Hide resolved
test/CMakeLists.txt Outdated Show resolved Hide resolved
up-cpp package will be used from conan package always
Updated how to build bundled build
lib/CMakeLists.txt Show resolved Hide resolved
conanfile.py Show resolved Hide resolved
conanfile.py Show resolved Hide resolved
updated conan package create command for conan 2.x
gregmedd added a commit to gregmedd/up-transport-zenoh-cpp that referenced this pull request Apr 15, 2024
GitHub will fall back to the cache from the target branch of a PR if it
fails to find a cache for the source branch of a PR. This causes issues
if the PR is changing the ci.yml file (see eclipse-uprotocol#32) without one of the
dependency repos changing (up-cpp and zenoh-c).

This change adds the SHA of the workflow file to the cache key so that
changes to ci.yml will invalidate the caches.

While this reduces the value of the caches when iterating on ci.yml
changes, a developer working in ci.yml would have the ability to remove
this temporarily if needed.
gregmedd added a commit to gregmedd/up-transport-zenoh-cpp that referenced this pull request Apr 15, 2024
GitHub will fall back to the cache from the target branch of a PR if it
fails to find a cache for the source branch of a PR. This causes issues
if the PR is changing the ci.yml file (see eclipse-uprotocol#32) without one of the
dependency repos changing (up-cpp and zenoh-c).

This change adds the SHA of the workflow file to the cache key so that
changes to ci.yml will invalidate the caches.

While this reduces the value of the caches when iterating on ci.yml
changes, a developer working in ci.yml would have the ability to remove
this temporarily if needed.
@gregmedd
Copy link
Contributor

The CI cache issue with the up-cpp step is fixed by #36. We will get that merged so you can rebase your PR onto the fix.

@gregmedd
Copy link
Contributor

Closing stale pull requests. We are tracking conan recipes on the 1.5.8 project board.

@gregmedd gregmedd closed this May 23, 2024
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.

None yet

3 participants