Rules to do cleanup

[romangg]

In GitLab by @erreur404 on May 6, 2020, 13:09

Hello,

I want to do some cleanup, what is the rule to do a merge request ?

One merge request for all class? one merge request by Class?

After : One commits for all cleanup by class? or one commit by type of cleanup/multiple commits by class ? or other ?

What is the rule to do the cleanup ? it can be good to add in Contributing file ;)

Another question, modify include files depends on which category?

[romangg]

I want to do some cleanup, what is the rule to do a merge request ?

Hey, that's great. And thanks for asking! :) I wouldn't say there can be a super precise rule set for that but let me give you some vague rules.

One merge request for all class? one merge request by Class?

That depends on what you change. If you change a single thing, like adding [[maybe_unused]] tags, then you can do this everywhere with a single commit. But if you change different things at once then it makes sense to divide at least by scope/file or something like that.

So split your commits up by logic or otherwise at least locality.

What is the rule to do the cleanup ? it can be good to add in Contributing file ;)

Yea, definitely. I want to improve the Contributing file soon anyway. Also adding information there about commit scoping.

Another question, modify include files depends on which category?

Hmm, I don't understand this question. What category do you mean?

[romangg]

In GitLab by @erreur404 on May 8, 2020, 13:04

Hmm, I don't understand this question. What category do you mean?

For example, delete include or delete unused include

[romangg]

Cleaning up includes can but put together in a single commit. Unused includes can be always removed.

But you can't test this by just removing them and seeing if it still compiles. Because it's often better to include something directly.

For example, let's say you include class_a.h in class_b.cpp. In class_a.h you include QRect which is also needed in class_b.cpp

Although it would compile without an additional include of QRect in class_b.cpp here it's better to also include QRect in class_b.cpp for clearness and so it doesn't break on refactors.

[romangg]

In GitLab by @erreur404 on May 24, 2020, 24:30

closed