0

I'm actually working on the web application which was made using technologies such as asp.net core, c#, angular, ef core. For the version control and CD we use Azure DevOps. As a team we try to follow Agile rules. When the developer starts the ticket, he creates a new branch forked from master and when he finishes he makes a Pull Request. Then the team members can have a look at the PR and add comments.

I'm thinking if it's a good behavior to removed unused directives (e.g. using blahnamespace.foo.bar) from a file which is not related to the pull request.

Just imagine a simple case. As a part of your task/ticket, you need to add some extra fields to class User plus some extra validation stuff. During the development, you found some unused directives in some other classes, so you decided to remove them to keep application code cleaner. If such a clean up is done for the single case then it's fine. The problem appears when the pull request is massive and tens of classes were modified. From my perspective, it's hard to make a code review because as a coder I want to check only the classes where the business logic was added or modified. Should it be forbidden to do such a clean up in classes which are not related to the task/ticket?

marked as duplicate by gnat, Community Apr 23 at 13:24

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

1

Just imagine a simple case. As a part of your task/ticket, you need to add some extra fields to class User plus some extra validation stuff. During the development, you found some unused directives in some other classes, so you decided to remove them to keep application code cleaner...

I'd suggest that the reason for your question is because you are asking how to handle the symptoms, rather than fixing the underlying problem.

So take a different approach. Write a Roslyn analyzer that reports an unused using as a compilation error (or eg use Resharper, which supports reporting them as errors too). That way, the developer needs to remove those unused usings before checking the code in. So the person who's making the change to User in your scenario won't be faced with the decision of whether to remove them or not as they won't be there.

There are automated tools available that can enforce clean code principles, so use them and you avoid those dilemmas of should someone tidy things up while making a functional change.

  • The idea is awesome, thanks! – GoldenAge Apr 23 at 13:23

Not the answer you're looking for? Browse other questions tagged or ask your own question.