Skip to content

Conversation

@shsms
Copy link
Contributor

@shsms shsms commented Sep 8, 2022

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't go through the code as it was imported, only looked at the supporting/tooling files.

I would also remove the extra channels sub-directory in both benchmarks/ and tests/, as channels should be the only package provided by this repo.

Only the comment about py.typed is important to at least check if it is working. The rest are minor, so approving.

@leandro-lucarella-frequenz leandro-lucarella-frequenz added this to the v0.1.0 milestone Sep 9, 2022
@shsms
Copy link
Contributor Author

shsms commented Sep 9, 2022

I would also remove the extra channels sub-directory in both benchmarks/ and tests/, as channels should be the only package provided by this repo.

this is also done.

@leandro-lucarella-frequenz
Copy link
Contributor

BTW, I'm trying to adopt a convention for "conversations" and using the 🚀 emoji to indicate for me we can resolve the conversation. I'm using it when I have a final comment because if I resolve it, then other people usually miss the comment because the conversation is collapsed. So if you see any conversations where my last message has a 🚀 emoji, feel free to mark it as resolved. If I don't have no further comments I will just resolve the conversations I started myself when whatever it was about is resolved.

@shsms
Copy link
Contributor Author

shsms commented Sep 9, 2022

That's a nice idea 🚀

@sahas-subramanian-frequenz sahas-subramanian-frequenz merged commit e9ca0e3 into frequenz-floss:v0.x.x Sep 9, 2022
@shsms shsms deleted the import-channels branch September 9, 2022 14:01
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.

3 participants