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

Adding procedural macros #459

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

eaglesemanation
Copy link

Adds multiple derive macros for implementing Arg, Get and Append traits combined. Makes it much less cumbersome to translate between dbus structs and rust structs. More details in this discussion: #456

Copy link
Owner

@diwic diwic left a comment

Choose a reason for hiding this comment

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

Hi and thanks for the PR! I've reviewed the code in our earlier discussions so the comments here are mostly about the formalia around it (docs, dependencies etc).

dbus-macros/Cargo.toml Outdated Show resolved Hide resolved
dbus-macros/Cargo.toml Outdated Show resolved Hide resolved
dbus/Cargo.toml Outdated Show resolved Hide resolved
Procedural macros for dbus crate
--------------------------------

Simplifies definition of complex dbus interfaces
Copy link
Owner

Choose a reason for hiding this comment

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

Umm, might need someting more here... And the main readme that lists all the crates in the workspace could advertise this crate as well.

Copy link
Author

Choose a reason for hiding this comment

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

I'll try to force myself to write some actually readable docs, but no promises =)

@@ -0,0 +1 @@
wip/**
Copy link
Owner

Choose a reason for hiding this comment

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

Not needed :-)

Copy link
Author

Choose a reason for hiding this comment

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

This is for trybuild dependency, it dumps stderr into wip/testname.stderr file if it can't find tests/ui/testname.stderr, so it's useful

@diwic
Copy link
Owner

diwic commented Mar 4, 2024

In addition, the tests fail so that needs to be fixed before merging also.

@diwic
Copy link
Owner

diwic commented May 22, 2024

@eaglesemanation Hi, how are things going? This is a really interesting PR, but once merged, I'm going to need your assistance with maintaining it, since procedural macros is not my area of expertise. If you're unavailable for long periods of time, that makes me a bit uneasy.

@eaglesemanation
Copy link
Author

Hey, sorry, life happened. First my dad came from overseas, needed to spend time with him, no clue when I'll meet him again. Now my employee wants me to relocate, and I need to make a decision pretty soon. I really hope I'll get more time soon to get back to this, but I'm not 100% curtain. If you're not comfortable with that - I could keep this as a separate crate, so any issues reported would be addressed to me

@diwic
Copy link
Owner

diwic commented May 29, 2024

Hey, sorry, life happened. First my dad came from overseas, needed to spend time with him, no clue when I'll meet him again. Now my employee wants me to relocate, and I need to make a decision pretty soon. I really hope I'll get more time soon to get back to this, but I'm not 100% curtain. If you're not comfortable with that - I could keep this as a separate crate, so any issues reported would be addressed to me

No worries. Life happens to all of us from time to time :-) Let me know how things are progressing and if/when you're more available again, we can have another shot at the upstreaming.

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

2 participants