search

kirsan31 / winforms-datavisualization

System.Windows.Forms.DataVisualization provides charting for WinForms applications.
MIT License
47 stars 19 forks source link

File cleanup only #14

Closed paul1956 closed 2 years ago

paul1956 commented 2 years ago

Only changed 1 project, changes in whitespace, remove unused usings and added endregion comment. If you can compare without whitespace there is very little difference.

kirsan31 commented 2 years ago

126 files changed, even with no white space it's 110 files changed 😲 It's insane to review such a massive PR. Pls do no more then 10 files per PR 🙏🙏🙏

paul1956 commented 2 years ago

@kirsan31 don't use Git to review these, there is no option when cleaning file except 1 file or everything in a project.

There are 2 changes outside of spacing, remove unused usings, fix endregion to correct incorrect formatting

It took less than 10 minutes to review every change with WinMerge.

It just seems like cleaning/reviewing 10 at a time it going to take a lot longer for both of us. If you disagree close PR.

kirsan31 commented 2 years ago

@kirsan31 don't use Git to review these, there is no option when cleaning file except 1 file or everything in a project. There are 2 changes outside of spacing, remove unused usings, fix endregion to correct incorrect formatting

It does not matter at all how to compare files, something else is important. In my practice, I have repeatedly run into terrible regressions after using automatic refactorings. Yes, they are rare, but they do happen... Any code change should be checked, especially in projects where there is no test coverage. Therefore, I initially did not automatically refactor all this solution, but decided, if possible, to refactor only those files that need real (not cosmetic) changes. And about whitespaces it's usual safe to ignore leading and trailing spaces, but errors with spaces inside code are more likely :(

It took less than 10 minutes to review every change with WinMerge.

We have 110 changed files with whitespaces changes ignored. Even if we discard what I said about whitespaces above and you open and review one file for 30 sec (and it's very fast, since there are enough changes even without whitespaces in each file), it's about one hour boring and tedious work. 10 minutes - you open and review every file for 5 sec???

It just seems like cleaning/reviewing 10 at a time it going to take a lot longer for both of us. If you disagree close PR.

Yes, as I said above, that's why I didn't do it initially. I do not see much point in this - why cosmetically refactor the code if no one will look into it? And if it does, then let it refactor...