I'm just going to cancel this thread. It seems like you want to go in a different direction, which is fine. I'm not a reviewer so I don't see what the point of reviewing your code would be.
Le lun. 13 juil. 2026 à 18:10, Morgan Smith <[email protected]> a écrit : > > Hello Earl! > > I apologize for the delay. Your most recent patch has now sat unreviewed for > almost a month. It was not my intention to let it sit that long. > > I also apologize for changing my mind a few times in this thread. This is my > first proper review and I am a bit of perfectionist. We have had more back > and > forth then is probably necessary due to my lack of experience. > > I have thought about this for a while and I have decided that presenting a > refined patch in response to someone's patch is perfectly reasonable (and just > FYI is something I've had reviewers do to me on many occasions). However, I > don't believe the language I used when presenting the patch in this thread was > ideal. It did indeed sound like I was "taking over" this issue. My intention > was to collaborate and thus in the future I will try to use language that > emphasizes that intention. > > I do stand by a pretty important idea that I have been trying to communicate > in > this thread: do 1 clear change per commit. You're still trying to do both the > API change over and reorganization in 1 commit. > > Moving some of the functionality of the monolithic function into > multiple, smaller, well documented functions could make the code more > readable. However, I'm not convinced your reorganization accomplishes > this. As I've said previously (although probably not communicated the > best), the reorganization can wait until a future thread where we take > advantage of some reorganization to implement some new features. > > To repeat: I do want the code reorganized. I just want to do that at a later > time when we're implementing new features. > > See attached two patches. > > The first one is the new tests you've submitted. I refactored them a little > but they are mostly untouched. They are good tests! Thank you very much for > creating them! > > > I added week, month and years tests > > I don't see any tests for year repeaters. Not strictly necessary but it would > be nice to have some. > > > In the future, it would be good to actually ... assert the correct errors > > are > > thrown for each type of malformed data. > > ERT has ways to check for the type of error (ex. calc does `(signal > 'math-overflow nil)`) but it does not have a way of checking the error message > as far as I know. I would also be interested in testing errors more > precisely. > > I've split the tests out into its own commit so we can run these new tests > before and after our changes are applied. > > The second commit is I believe actually identical to the first patch I sent > you. Using the following git command you can see the logic and ordering is > the > same while only switching out the API > > git show --word-diff=color --word-diff-regex='[^[:space:]()]+' > --diff-algorithm=histogram HEAD > > If you're happy with those two commits, then I can apply them and we can close > out this thread. The commits have already passed my fairly comprehensive > local > CI system. > > Tests pass after each patch on emacs 30.2. > Tests pass after final patch on emacs 28 and 29 and a recent emacs/master > build. > Tests pass after final patch with TZ set to UTC, Europe/Istanbul, and > America/New_York. > Tests pass after final patch with the locale set to C, en_US, fr_FR, de_DE, > and > he_IL. > > > Is there any reason specifically that I should use `pcase' over `cl-case'? > > It was just a nitpick. cl-case is a simpler function that does exactly > what you where trying to do. Logically the solutions are identical. I > would imagine cl-case is faster but I haven't tested it. > > > I really do appreciate the work you've put into this. Overall you've > submitted > some really high quality patches! I do hope we can continue to work together > on some things. > > I'm sorry this has dragged on so long and I appreciate your patience as I > learn > some communication skills and develop my standards for contributions. You've > helped me in many ways in this thread :) >
