-
Notifications
You must be signed in to change notification settings - Fork 13
Remove unused challenges 'max_scores' on create [HIRO-270] #139
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
Remove unused challenges 'max_scores' on create [HIRO-270] #139
Conversation
Only commented it out instead of deleting to: 1. Indicate the reasoning why the proto is skipping a field number/tag. 2. In case we want to implement it back in the future and actually make use of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the unused max_scores field from the challenge creation API. The field was previously defined in both the protobuf schema and OpenAPI specification but is no longer needed.
Changes:
- Removed
max_scoresfield fromChallengeCreateRequestmessage in protobuf schema - Removed
max_scoresfield from OpenAPI schema definition - Added
claim_rewardboolean field to another schema (appears unrelated to main purpose)
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| hiro.proto | Removed unused max_scores field and its documentation comment from ChallengeCreateRequest |
| hiro-openapi.yml | Removed max_scores field definition from challenge creation schema and added claim_reward field to a different schema |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@sesposito can you review this one and keep in mind the requirements we had from Halfbrick? 🙏 |
|
Just wondering, can we be sure this will not break any client code? |
|
@VBorisof you raise a fair point, the proto json unmarshaler in Hiro is initialized with: |
| // Deprecated: This field is no longer used and will be removed in future versions. | ||
| // Maximum number of scores a user can submit to the leaderboard. | ||
| int64 max_scores = 6; | ||
| int64 max_scores = 6 [deprecated = true]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Codegen doesn't currently pick this up right? (the deprecated = true bit)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can work on adding support for deprecated, I want to make changes in hiro leaderboard that will need deprecated but that will be a new pr, created linear ticket
https://linear.app/heroiclabs/issue/HIRO-331/add-support-for-deprecated-annotations-in-port
Removed the unused
max_scoresfield from the challenge creation request and schema.