My situation is this: I am contributing to a Symfony/PHP project. I had to completely reskin one page in the application. The controller logic is almost totally unchanged. I only had to change the template file (named index.html.twig). But the changes are completely different. If I make a pull request with these changes against the RC branch, the diff for this pull request will be all over the place. For some reason this seems wrong to me. Since this template file is a "new" change, I'm currently getting around this issue by creating a new file named "index_new.html.twig" and changing the controller logic to reference this file. That way when I make a pull-request, the template file will be displayed as a new file and it will be obvious that the controller references the new file. But this also seems wrong because it seems to be a hacky solution. Also, what if this page is redesigned again? I might have to do "index_new_new.html.twig".

What is the optimal way to deal with this situation?

  • 1
    The general strategy is to commit small, cohesive changes. Commit 1: Bugfix for functionX; Commit 2: Updated theme. That said,if it's a big PR, it's a big PR. Diffs are a tool, which just won't be useful for comparing the changes here. It can still be reviewed. – bitsoflogic Nov 8 '18 at 19:26
  • 7
    Regardless, do not append "_new" to files. I've seen files named "new" that were 10 years old. Try to use meaningful names like "stellar_theme.html.twig". – bitsoflogic Nov 8 '18 at 19:30
  • The reviewers will tell you. – Goyo Nov 8 '18 at 19:34

Take a look at the code base to see what the prefered layout is. Or even ask the reviewers if you can on their forum/email/im. That way your contribution is consistent.

As for the size. Does it pass the tests? And is the change cohesive?

If you broke the tests, your change probably won't be accepted either way you dice it. If you fix the tests, make sure those changes are easy to understand. They might even guide you as to whether the change is cohesive or not.

If the change isn't cohesive find the smaller cohesive changes that make it up. Break your commit down and offer each as a separate pull.

At the end of the day, this isn't about pretty diffs, but about clear communication and understanding. If the diffs are messy, but the tests clear, and your commit comment direct - that is way better than - a one line change, cryptic tests, and a throw away comment.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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