-
Notifications
You must be signed in to change notification settings - Fork 54
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
fix(types): SentinelWeapons typing #485
Conversation
Also a fix for inconsistency mentioned in #482 |
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.
SentinelWeapons
is aProductCategory
SentinelWeapons
is a valid entry to thecategories
array, but is aProductCategory
I see that the productCategory is defined by DE, so I'll leave it as a productCategory. But since it's also a category, it's not properly typed as a category. I will make the update and see if that's more acceptable. |
Since SentinelWepeapons is a category and productCategory, the type SentinelWeapon will reflect that.
that's fine if you do that on your branch, but i'm not going to merge it. |
Can I ask why? There's literally bugs in the code. |
All the change is, is changing the SentinelWeapon type category from "Primaries" to "SentinelWeapon" (you said it was wrong yourself) and making SentinelWeapon a proper type for category. |
Because I said it shouldn't be changed. Yes, I'm annoying, sorry. My point is you're crossing streams of a ProductCategory into Category, and I'm telling you I don't want it. It's also not "literally bugs in the code", considering you're playing with types, you're not even fixing build code |
This change will break more then it fixes so at the time being this isn't a critical fix |
SentinelWeapons was made a Category and ProductCategory by design so I would say those streams were already crossed lol. I'll just do my own data repository, this project seems to be causing me more headaches than what it's worth. |
it's literally not a category. it's an option for the category argument because there's a file created for it. that's not the same as a member of the Category type |
What did you fix?
SentinelWeapons was incorrectly typed as it was not typed as a Category and should be as it behaves similarly to "Primary" and "Secondary" categories.
Reproduction steps
Evidence/screenshot/link to line
Considerations