Fixes space vs tab.
Details
Diff Detail
- Repository
- rS FreeBSD src repository - subversion
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 17190 Build 17040: arc lint + arc unit
Event Timeline
Please seperate this into 2 reviews, one for the white space/tab cleanup and one for the SPDX tag addition.
Thank you
I think it is unnecessary, it will give unnecessary extra work and as I have touched each file the changes can be done at once as it is more cosmetic and there is no functional changes.
Cheers,
It is normal to separate all whitespace changes from other. It is preferable to separate all SPDX changes from others when it is sweeping. I strongly assert that as the person now cleaning up a 180k lines and 16 commits of diff involving SPDX tags that this separation occur before this is commited.
Sorry, I strongly disagree, I would need to make two reviews and two commits that can be done at once and there is no functional changes.
The commit log easily can describe these changes:
- Fix style(9) space vs tab.
- Add SPDX tag on those files that didn't have it.
I'm inclined to listen to the reviewers advice that I added, if that is mandatory, I will do, otherwise I can't see any problem to have these changes in one commit.
I agree with @rgrimes here. Separating out changes makes it much easier to merge later on as well as deal with merge conflicts. While we may not have many, there are also downstream consumers of our code that would appreciate small, self-contained changes.
I like the space cleanup, thanks, and SPDX part is also fine. But I also agree that mixing together unrelated changes is not a very good idea. While in this particular case I would personally not create much churn about it (much worse when formatting changes are mixed with code changes, which is not really a case here), once other people complained, I think Marcelo you could faster do the requested separation rather then argue indefinitely from weak position.
I'm aware of that! I'm in contact with the author of that part and will add LICENSE soon.
Tks,
FWIW, I sometimes post "combined" patches for review and once it is approved split it up into commits. It looks like you've already pared this down, but as general idea I don't think that's terrible for review so long as the commits are split up.
I think the whitespace changes are generally ok.