Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up misguided legacy changes #4266

Open
jgonggrijp opened this issue Jul 19, 2023 · 1 comment
Open

Clean up misguided legacy changes #4266

jgonggrijp opened this issue Jul 19, 2023 · 1 comment

Comments

@jgonggrijp
Copy link
Collaborator

In the distant past there have been changes of questionable value, often breaking, often complicating and/or pessimizing the code, and sometimes removing pre-existing tests. I consider #3843 a good example of this type of change. I'm sure that example is one of many, because I encountered similar changes when I triaged the open issues and pull requests in stage 1 of #4244. In addition, I have seen many similarly unfortunate changes in Underscore, often (but not always) by the same author.

Many small changes make a big difference, and I suspect that hunting down and reverting as many of these changes as possible would slim down and speed up the code considerably. Unfortunately, this is easier said than done, for three main reasons:

  • Finding the changes will be challenging in the first place.
  • Since most of these changes were done at least seven years ago, they tend to have been buried under later changes, which makes reversal nontrivial.
  • Since many of these changes were breaking, reverting them will be a breaking change again.

To address this, I would like to do the following:

  1. Curate a list of changes that should be reverted, in the current issue ticket. This is, of course, subject to discussion. Suggestions for candidates are very welcome.
  2. Postpone reverting the changes until a major version update is near.
  3. Revert the changes from the list in the reverse order of application, on a future branch.
  4. Base the new major version on the future branch.

Candidate list (to be updated as the discussion progresses):

@jgonggrijp
Copy link
Collaborator Author

#3758, while by itself never merged, is a treasure trove for discovering this type of change. Right in the opening post, it already lists #3659, #3644 and #1407. When I get to it, I want to follow those references in order to potentially find even more of those.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant