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.

Sign up for GitHub

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

Jump to bottom

Add Rust Test Agent #23

Merged
merged 13 commits into from
May 15, 2024
Merged

Add Rust Test Agent #23

merged 13 commits into from
May 15, 2024

Conversation

matthewd0123
Copy link
Contributor

  • Add changes to TCK to support Rust Test Agent for Register_and_Send and register_and_unregister

@matthewd0123 @sagarsshah
Add Rust Test Agent
eb0531e 
* Add changes to TCK to support Rust Test Agent for Register_and_Send and register_and_unregister

Co-Authored-By: sagarsshah <157176449+sagarsshah@users.noreply.github.com>
@neelam-kushwah neelam-kushwah self-requested a review April 12, 2024 21:25
PLeVasseur
PLeVasseur reviewed Apr 15, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
PLeVasseur
PLeVasseur requested changes Apr 15, 2024
View reviewed changes
Copy link
Contributor

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

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

Hey @sagarsshah and @matthewd0123 --

Could you please:

  • take a look at the project restructuring suggestions I made here
  • run cargo fmt
  • run cargo clippy
    • resolve any issues it points out w/o using the allow syntax

And ping me when you have a chance?

PLeVasseur
PLeVasseur reviewed Apr 15, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
PLeVasseur
PLeVasseur reviewed Apr 15, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
PLeVasseur
PLeVasseur reviewed Apr 15, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
PLeVasseur
PLeVasseur reviewed Apr 15, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
@matthewd0123 @PLeVasseur
Changes based on review
02e09c9 
* update licenses
* remove get_ustatus_code
* add err_string to reuse error msgs
* change signature of parse_uauthority
* remove commented code
* formatting on lib.rs
* remove panic references since doesn't panic

Co-Authored-By: Pete LeVasseur <11622119+plevasseur@users.noreply.github.com>
@matthewd0123
Few more comments
8c8762a 
* Change specialfields to default
* Add "not implemented" to notification type
* Some fixes to debug msgs and comments
PLeVasseur
PLeVasseur reviewed May 15, 2024
View reviewed changes
test_agent/rust/src/utils.rs Outdated
error!("Error: traceparent is not a string");
};

uattributes.special_fields = SpecialFields::default();
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh... so did initializing uattributes with Default not set special_fields...? If so, how annoying...

@matthewd0123
Remove initialization of special_fields
cb89638 
* Remove special_fields from the uattributes deserializer
PLeVasseur
PLeVasseur approved these changes May 15, 2024
View reviewed changes
Copy link
Contributor

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

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

It's been a long road to get here, but I am happy to write...

LGTM!!

🎉

@neelam-kushwah neelam-kushwah merged commit c15d274 into eclipse-uprotocol:main May 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagarsshah sagarsshah sagarsshah left review comments

@PLeVasseur PLeVasseur PLeVasseur approved these changes

@neelam-kushwah neelam-kushwah Awaiting requested review from neelam-kushwah

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@matthewd0123 @neelam-kushwah @PLeVasseur @sagarsshah