Skip to content

Conversation

@maloel
Copy link
Contributor

@maloel maloel commented Dec 7, 2022

  • Rename utilities -> rsutils (project, paths, namespaces)
  • Pyrsutils now shows with this name and not utilities-py
  • Rename librealsense::to_string() (out of types.h) and rs2::to_string() (out of rendering.h) and move into rsutils::string::from()
  • Python libs now go under Library/Python (they were intermingled with the other libraries under Library)

@Nir-Az
Copy link
Collaborator

Nir-Az commented Dec 8, 2022

concurrency.h in not inside a namespace.
Can you add an rsutils namespace for it?

@Nir-Az
Copy link
Collaborator

Nir-Az commented Dec 8, 2022

chrono.h is also without a namspace, is that on purpose?
Doesn't it possible that it will interfere with users code?


using namespace utilities::time;
using namespace rsutils::time; // from rsutils/
using namespace utilities::time; // from common/
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have 2 time classes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not two classes.
We had one namespace, and the stuff in common/ added to it.
I didn't touch common/, so it stayed in the utilities namespace, while the stuff in rsutils moved.
common/ adds device-specific (L500 mainly) logic, which doesn't belong in rsutils.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK.. I must say it's confusing.

@maloel
Copy link
Contributor Author

maloel commented Dec 8, 2022

concurrency.h in not inside a namespace. Can you add an rsutils namespace for it?

I can and I will.
You want it all in one PR?

@maloel
Copy link
Contributor Author

maloel commented Dec 8, 2022

chrono.h is also without a namspace, is that on purpose? Doesn't it possible that it will interfere with users code?

This is on purpose, at least in part -- here we need to override global mechanisms. Still, I'll take a look later.

@Nir-Az
Copy link
Collaborator

Nir-Az commented Dec 8, 2022

concurrency.h in not inside a namespace. Can you add an rsutils namespace for it?

I can and I will. You want it all in one PR?

On the next one :)

@Nir-Az Nir-Az self-requested a review December 8, 2022 14:16
@maloel
Copy link
Contributor Author

maloel commented Dec 8, 2022

Thankyousir

@maloel maloel merged commit 0211035 into IntelRealSense:development Dec 8, 2022
@maloel maloel deleted the types branch December 8, 2022 14:18
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.

2 participants