Skip to content

feat/docs caravan health privacy#195

Closed
AvhiMaz wants to merge 3 commits intocaravan-bitcoin:mainfrom
AvhiMaz:feat/docs-caravan-health-privacy
Closed

feat/docs caravan health privacy#195
AvhiMaz wants to merge 3 commits intocaravan-bitcoin:mainfrom
AvhiMaz:feat/docs-caravan-health-privacy

Conversation

@AvhiMaz
Copy link

@AvhiMaz AvhiMaz commented Mar 18, 2025

No description provided.

@changeset-bot
Copy link

changeset-bot bot commented Mar 18, 2025

⚠️ No Changeset found

Latest commit: c2664bd

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Mar 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
caravan-coordinator ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 26, 2025 6:04am

@bucko13
Copy link
Contributor

bucko13 commented Mar 19, 2025

Hi @AvhiMaz. Thanks for the contributions! Could you please split up the commit history of this PR so it only has the relevant code? The title says docs for caravan health but it includes #194 code.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like having this documentation in jsdocs. What's the motivation for adding this file as well? I don't mind it but we don't have this anywhere else and my biggest concern is that it will be hard to maintain. Ideally we just have more jsodcs closer to the code like you've added and then we can start generating these docs automatically.

Copy link
Author

Choose a reason for hiding this comment

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

The motivation for adding this file was to provide a structured overview of these metrics in one place, making it easier to reference their definitions, calculations, and expected ranges. However, I understand the concern about maintainability. I’m happy to keep everything in JSDoc instead and work towards generating the docs automatically. Let me know if you'd like me to remove this file and focus on enhancing the inline documentation instead.

Signed-off-by: AvhiMaz <avhimazumder5@outlook.com>
Signed-off-by: AvhiMaz <avhimazumder5@outlook.com>
@AvhiMaz
Copy link
Author

AvhiMaz commented Mar 19, 2025

@bucko13 please. have a look I've made suggested changes, Thank you!

- UTXO Fragmentation (1 input, more than 2 standard outputs)
- Consolidation (more than 1 input, 1 output)
- CoinJoin or Mixing (more than 1 input, more than 1 output)
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

i still think it's worth keeping these descriptions as they are pretty useful. Once we have doc generation this can all be put in a single document (stripped of the code part for readability).

Signed-off-by: AvhiMaz <avhimazumder5@outlook.com>
"prettier": "^3.1.1",
"turbo": "^2.0.3"
"turbo": "^2.0.3",
"typedoc": "^0.28.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to add this yet until we're actually using it in CI. We can do that separately.

@bucko13
Copy link
Contributor

bucko13 commented Apr 2, 2025

I'm not really sure what the point of this PR is. It looks like some very basic cleanup and now adding a package that's not even being used yet. If you wanted to experiment with generating documentation that we can build and hopefully host with CI I'd be open to work on that (especially something more at the top monorepo level).

@AvhiMaz
Copy link
Author

AvhiMaz commented Apr 2, 2025

@bucko13 closing the pr, let me focus on other one, will be seeing it, implement it correctly and will raise a pr again. thank you.

@AvhiMaz AvhiMaz closed this Apr 2, 2025
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