Avalonia
Avalonia copied to clipboard
AvaloniaUI
feat: Enable Rule CA1822
What does the pull request do?
CA1822: Mark members as static
What is the current behavior?
What is the updated/expected behavior with this PR?
How was the solution implemented (if it's not obvious)?
Checklist
- [ ] Added unit tests (if possible)?
- [ ] Added XML documentation to any related classes?
- [ ] Consider submitting a PR to https://github.com/AvaloniaUI/Documentation with user documentation
Breaking changes
Obsoletions / Deprecations
Fixed issues
Part of #8944
You can test this PR using the following package version. 11.0.999-cibuild0024739-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]
I'm strongly against this one. It's too many changes and the decision to make things static or not should have been made when the code was first written -- not by another person doing many at once.
This type of thing just shouldn't be done... it changes the API surface in ways you may not understand and may not make sense on a case by case basis.
This type of thing just shouldn't be done... it changes the API surface in ways you may not understand and may not make sense on a case by case basis.
I'm not entirely true, I just changed the private and internal methods.
I'm strongly against this one. It's too many changes and the decision to make things static or not should have been made when the code was first written -- not by another person doing many at once.
You're right, that's why it's marked as bezzo. Hope the original authors can do the review.
I think version 11 is an unmissable opportunity to make Avalonia even better. After 11 is released as final, we will have to wait a long time before we take an opportunity like this.
You can test this PR using the following package version. 11.0.999-cibuild0024874-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]
This type of thing just shouldn't be done... it changes the API surface in ways you may not understand and may not make sense on a case by case basis.
I'm not entirely true, I just changed the private and internal methods.
If so, then it doesn't need to be done before the 11.0 API is locked -- it can be done anytime. Actually, I still think it's better to ignore this rule and only update these things when there is a reason on a case-by-case basis.
Fundamentally the rule states what "Can" be made static. It doesn't say what "Should" be made static. That is an important distinction we would have to review on a case-by-case basis. Which really seems like a lot of time honestly and difficult without original authors reviewing their parts of the code. If you can get the original code authors to comment obviously I have no concerns.
I think version 11 is an unmissable opportunity to make Avalonia even better. After 11 is released as final, we will have to wait a long time before we take an opportunity like this.
I agree in concept we want to make 11 as good as possible. And API changes are much more difficult after that. I also appreciate the time you are taking going through these sorts of issues. However, I would be much more strategic and only change these types of things when there is a reason. Since this isn't public API too, there is even less reason to do it now before 11.0. I would still ignore this rule.
You can test this PR using the following package version. 11.0.999-cibuild0025013-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.0.999-cibuild0025118-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]
You can test this PR using the following package version. 11.0.999-cibuild0025842-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]