4 ms·

Nonrecursive Lisp Reader

39 points by eklitzke 4y ago

WalterGR 4y ago
> Showing 4 changed files with 1,229 additions and 989 deletions. Am I interpreting it correctly that this page shows a single commit??
- deleted 4y ago
  [deleted]
- avgcorrection 4y ago
  This looks like three squashed commits based on the formatting of the commit message. I guess from a PR. The “src/lread.c (readevalloop)” commit could in turn be based on many smaller commits.
  - WalterGR 4y ago
    Well that’s just gross!
    - chrsig 4y ago
      What part of it? and why?
      - WalterGR 4y ago
        IMO this is a perfect example of squashing destroying history and leading to unintelligible commits.
        aseipp 4y ago
        It's actually a perfect example of how people don't understand others' development practices and don't understand the history their own field came from. This single commit is exactly how the original patch was authored, as you can see in the bug tracker ticket number mentioned in the issue, the original patch is basically the same. The formatting of the commit message has nothing to do with whether it was squashed by any tools; the "format" used in the commit message is the same as the GNU "CHANGELOG" format used in many early GNU projects (which itself I think was just a formalisation of early RCS changelogs), and the CHANGELOG format actually suggests that you individually document what changes you made to each file. This is done so that even if the entire source control system is changed, you can just look at the CHANGELOG to understand when and where things happened. You can go look up "GNU Changelog style" and find more about this. Not only that, it is very common for patches for projects like Emacs to reach +1,000 LOC deltas. It is actually very common for many projects to have large, individual patches written by experienced contributors that are then reviewed likewise by experienced contributors. It may startle you, but many people actually don't like reading 50 individual 10 line patches at a time, and many people don't care to add lots of commits to their project when the greater overall thrust of the change is more important and can be encapsulated with a single change. [1] Not only are you wrong, but you should try actually understanding other peoples projects before you start running on about how "unintelligible" they are and calling them gross. [1] Emacs does not exist in the realm of GitHub; it very much does things "the old way" by using a bug tracker and posting patch files to each other, and one of the implications of that is that large .patch files are better than a litany of small ones. Anyone who has had to pull .patch files off a bug tracker would probably agree with me I guess; but I like bigger patches anyway whether it's GitHub or not.
        morelisp 4y ago
        It's also obviously one logical change, it makes no sense to make a parser "half non-recursive".
    - morelisp 4y ago
      It should be broken down into a subtask per token type to minimize story points per work item!
deleted 4y ago
[deleted]
deleted 4y ago
[deleted]
morelisp 4y ago
Does anyone have some more detailed numbers about the performance improvements? https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55676 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55676 says "a few percent" but I'm more curious with regards to what language constructs, and whether it's mostly from the removal of recursion or the other "closer attention to performance."

39 points by eklitzke 4y ago

> Showing 4 changed files with 1,229 additions and 989 deletions. Am I interpreting it correctly that this page shows a single commit??

deleted 4y ago
[deleted]
avgcorrection 4y ago
This looks like three squashed commits based on the formatting of the commit message. I guess from a PR. The “src/lread.c (readevalloop)” commit could in turn be based on many smaller commits.
- WalterGR 4y ago
  Well that’s just gross!
  - chrsig 4y ago
    What part of it? and why?
    - WalterGR 4y ago
      IMO this is a perfect example of squashing destroying history and leading to unintelligible commits.
      - aseipp 4y ago
        It's actually a perfect example of how people don't understand others' development practices and don't understand the history their own field came from. This single commit is exactly how the original patch was authored, as you can see in the bug tracker ticket number mentioned in the issue, the original patch is basically the same. The formatting of the commit message has nothing to do with whether it was squashed by any tools; the "format" used in the commit message is the same as the GNU "CHANGELOG" format used in many early GNU projects (which itself I think was just a formalisation of early RCS changelogs), and the CHANGELOG format actually suggests that you individually document what changes you made to each file. This is done so that even if the entire source control system is changed, you can just look at the CHANGELOG to understand when and where things happened. You can go look up "GNU Changelog style" and find more about this. Not only that, it is very common for patches for projects like Emacs to reach +1,000 LOC deltas. It is actually very common for many projects to have large, individual patches written by experienced contributors that are then reviewed likewise by experienced contributors. It may startle you, but many people actually don't like reading 50 individual 10 line patches at a time, and many people don't care to add lots of commits to their project when the greater overall thrust of the change is more important and can be encapsulated with a single change. [1] Not only are you wrong, but you should try actually understanding other peoples projects before you start running on about how "unintelligible" they are and calling them gross. [1] Emacs does not exist in the realm of GitHub; it very much does things "the old way" by using a bug tracker and posting patch files to each other, and one of the implications of that is that large .patch files are better than a litany of small ones. Anyone who has had to pull .patch files off a bug tracker would probably agree with me I guess; but I like bigger patches anyway whether it's GitHub or not.
        morelisp 4y ago
        It's also obviously one logical change, it makes no sense to make a parser "half non-recursive".
  - morelisp 4y ago
    It should be broken down into a subtask per token type to minimize story points per work item!

deleted 4y ago

[deleted]

deleted 4y ago

[deleted]

morelisp 4y ago

Does anyone have some more detailed numbers about the performance improvements? https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55676 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55676 says "a few percent" but I'm more curious with regards to what language constructs, and whether it's mostly from the removal of recursion or the other "closer attention to performance."