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?

New contributor
Kodos Johnson is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
  • 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 at 19:26
  • 3
    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 at 19:30
  • The reviewers will tell you. – Goyo Nov 8 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.

New contributor
Kain0_0 is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.

Your Answer

Kodos Johnson is a new contributor. Be nice, and check out our Code of Conduct.
 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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