-
Notifications
You must be signed in to change notification settings - Fork 4
TypeScript library Generation #107
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
Conversation
…xisting library under python/ and auto-generate openapi schema that powers typescript library generation.
… the API contracts between python and typescript
…ate thresholds are met (greater than 3 files and/or greater than 50 lines of code), note this for AI-automated-code-generation
…ion logic" This reverts commit 6a06d9b.
|
@mphuff Haven't had a chance to look at the code in detail but so far I think I'm okay with this idea. I want to double check that publishing to Pypi and the readthdocs stuff still work as expected (not sure if you are able to validate that or not). Should we plan to add a docs site for the TS version of the code as well? Do you need to add a workflow for publishing to npm? |
|
Also, make sure you account for some of the crappier edges of the data model, like active_time_seconds from the v2 workout model and maxHr from the telemetry # from workout.py
if v2_workout:
data["active_time_seconds"] = v2_workout.active_time_seconds
telemetry: dict[str, Any] | None = data.get("telemetry")
if telemetry and "maxHr" in telemetry:
# max_hr seems to be left out of the heart rate data - it has peak_hr but they do not match
# so if we have telemetry data, we can get the max_hr from there
data["details"]["heart_rate"]["max_hr"] = telemetry["maxHr"]Also, i think we can improve some of the python scripts by using uv, but that can be something to add later. |
|
I plan to make sure all python scripts can be leveraging Happy to discuss further though if this is something you want in the core library; I'm also ok if you think it muddies the cleanliness too much and I can maintain my own copy but keep me honest. You're the owner so want you to have the final say in it. My general approach here:
|
Docs site for TS yes - I incorporated |
This reverts commit d9501a9.
a48ad5a to
35a1db1
Compare
292a2b7 to
e697d2a
Compare
…ypescript generation
@NodeJSmith - After working through this more, I'm wondering if you have a strong opinion here or not. I had originally thought about having Claude just auto-generate the TypeScript library on change. More I think about it, I think I'm talking myself out of this approach and rather than combining into libraries, I'll maintain the My problem I'm having is it doesn't know when to just make edits vs. cut whole files out as the library evolves and context grows; and I don't know that I want to continue to engineer much more on this vs. get back to actual feature development in the products :) |
|
@NodeJSmith - Closing this PR in favor of a new one I'll be opening up. It's a much cleaner overall implementation and I think the more maintainable version longer-term. |
@NodeJSmith - i'm aiming to have a new PR this weekend. The only change in your repository would be generating the actual schema asset that would be part of the release (and I will have a PR for this). The TS library that I will check in will cross validate the generated records from the Python library and the TS library as an integration test. I'm working out a few remaining edge cases between the initial implementations, but I am nearly there; just haven't had a lot of time this week. This is a much cleaner and stable implementation, and it actually ties the underlying TS library for a variety of pieces to your released version. This is both for pulling in the generated model file as well as running a python integration test to pull real data via your released library asset. My maintenance plan will be to run the diff between releases through [name my favorite AI tool of the week] and commit my own PR and release to npmpm. Your library should remain the source of truth and will be credited accordingly. |
|
That sounds good to me, I'll try to get to the review quickly so as not to slow you down 👍 |
Opening this PR right now for validation of the automation as well as to get initial feedback.