Conversation
IoanAndrasi
left a comment
IoanAndrasi
left a comment
There was a problem hiding this comment.
Great work for initial work, what I would also mention is that some files are very well documented with comments while others lack's comments.
| id: claude-review | ||
| uses: anthropics/claude-code-action@v1 | ||
| with: | ||
| claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} |
There was a problem hiding this comment.
Who is providing this secret key ?
If token usage is hit maybe we need another secret key as backup ?
There was a problem hiding this comment.
We will get copilot support, not sure when.
| } | ||
|
|
||
| struct TopologyInner { | ||
| state: RwLock<TopologyState>, |
There was a problem hiding this comment.
Shouldn't we use tokio::sync::RwLock for all locks which is more ideal for async context ?
std::sync::RwLock blocks the threads which increase the risk of task starvation.
There was a problem hiding this comment.
Thanks for your remark. As long as we do not hold the lock across async points we are safe, rust type system will catch this anyhow. The code is correct.
But you triggered some thoughts whether we can get rid of Arc since we hold a RW lock. Let me come back to you once the idea is settled to show you an alternative.
2 participants
Summary
Incubator for OpenSOVD Core
Checklist
Related
Notes for Reviewers