Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Enforced all *Info fields being public readonly #20011

Merged
merged 2 commits into from Apr 26, 2022
Merged

Enforced all *Info fields being public readonly #20011

merged 2 commits into from Apr 26, 2022

Conversation

penev92
Copy link
Member

@penev92 penev92 commented Apr 26, 2022

This came up during work on #19948.

@penev92
Made all traitInfo fields readonly
aa184af 
This came up while working on the new documentation generation and comparing the results to ORAIDE's own code parser.
@penev92
Made all projectile and warhead fields readonly
591cbb8 
This came up while working on the new documentation generation and comparing the results to ORAIDE's own code parser.
@RoosterDragon
Copy link
Member

Should we be enforcing this in a linter somewhere?

@penev92
Copy link
Member Author

penev92 commented Apr 26, 2022 •
edited

I don't think we can?
Non-public and/or non-readonly fields are still valid for not-YAML-exposure purposes. And we don't have 100% DescAttribute coverage for that to be the deciding factor. (if we had that we could say "all fields with DescAttribute are destined for exposure to YAML and thus must be public readonly", but alas...)
So I don't see good enough criteria for linter enforcement (I like the idea though).

@abcdefg30 abcdefg30 merged commit 8651561 into OpenRA:bleed Apr 26, 2022
@penev92 penev92 deleted the readonlyFields branch September 2, 2022 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RoosterDragon RoosterDragon RoosterDragon approved these changes

@abcdefg30 abcdefg30 abcdefg30 approved these changes

Assignees
No one assigned
Labels
PR: Needs +2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@penev92 @RoosterDragon @abcdefg30