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

Remove deprecated header for public api #542

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

xuzhenbao
Copy link
Contributor

fix #538
1.Remove the deprecated headers used by rsa_spi
2.Remove the deprecated headers used by pubsub_spi
3.Remove the deprecated headers used by conan_test_package
4. add conan_test_package for rsa_discovery_zeroconf.

@codecov-commenter
Copy link

codecov-commenter commented May 5, 2023

Codecov Report

Merging #542 (d84bf74) into master (c0666d0) will decrease coverage by 0.03%.
The diff coverage is 33.33%.

❗ Current head d84bf74 differs from pull request most recent head cdc77e0. Consider uploading reports for the commit cdc77e0 to get more accurate results

@@            Coverage Diff             @@
##           master     #542      +/-   ##
==========================================
- Coverage   77.43%   77.41%   -0.03%     
==========================================
  Files         225      225              
  Lines       34532    34523       -9     
==========================================
- Hits        26739    26725      -14     
- Misses       7793     7798       +5     
Impacted Files Coverage Δ
...bsub/pubsub_admin_udp_mc/src/pubsub_udpmc_common.c 0.00% <0.00%> (ø)
...sub_admin_udp_mc/src/pubsub_udpmc_topic_receiver.c 0.00% <0.00%> (ø)
...lizer_avrobin/src/pubsub_avrobin_serializer_impl.c 5.65% <0.00%> (+0.01%) ⬆️
...b/pubsub_utils/src/pubsub_serialization_provider.c 67.08% <ø> (ø)
...ubsub/pubsub_utils/src/pubsub_serializer_handler.c 93.47% <ø> (ø)
...te_service_admin_shm_v2/rsa_shm/src/rsa_shm_impl.c 96.89% <ø> (ø)
...ubsub_admin_udp_mc/src/pubsub_udpmc_topic_sender.c 57.31% <20.00%> (+0.76%) ⬆️
...ubsub_serializer_json/src/pubsub_serializer_impl.c 35.53% <46.66%> (-0.11%) ⬇️
...ervices/rsa_rpc_json/src/rsa_json_rpc_proxy_impl.c 96.75% <100.00%> (ø)
libs/framework/src/module.c 74.36% <100.00%> (+0.72%) ⬆️

... and 4 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@pnoltes pnoltes left a comment

Choose a reason for hiding this comment

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

Overall LGTM, with exception of the hashmap changes for the services.

These are backwards incompatible changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Replacing the hashmap for a service is a backwards incompatible update. I think we should postpone that until Celix 3.0.0

@@ -29,6 +29,7 @@

#include "endpoint_listener.h"
#include "remote_service_admin.h"
#include "celix_long_hash_map.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

Replacing the hashmap for a service is a backwards incompatible update. I think we should postpone that until Celix 3.0.0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK,Let's merge it until Celix 3.0.0

@PengZheng
Copy link
Contributor

I agree with @pnoltes. Let's merge it for 3.0.0.

@PengZheng
Copy link
Contributor

Now it is good time to re-attempt this PR. @xuzhenbao

@PengZheng
Copy link
Contributor

Given that pubsub has already been removed, it this PR already done? @xuzhenbao

@xuzhenbao
Copy link
Contributor Author

Given that pubsub has already been removed, it this PR already done? @xuzhenbao

Thanks for the reminder. Maybe I need to plan a time to update this PR.

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.

Remove remaining use of deprecated headers in public api
4 participants