[GH-ISSUE #42] The loop's condition feels off #17

New issue

kerem · 2026-03-02T02:53:02+03:00

kerem commented

2026-03-02 02:53:02 +03:00

Originally created by @djalilhebal on GitHub (Oct 23, 2023).
Original GitHub issue: https://github.com/artiebits/fake-git-history/issues/42

First of all, nice utility.

I just checked its source code and the loop's condition felt off.
The end result will be fine (always < max commits per day), but it seems as if it's "changing its mind"...
Imagine the following execution of i < getRandomIntInclusive(0, 2):

First iteration: 0 < 1
Second iteration: 1 < 2
Third iteration: 2 < 0

A simple edit should make the intention clearer:

-    for (let i = 0; i < getRandomIntInclusive(...commitsPerDay); i++) {
+    const commitsCount = getRandomIntInclusive(...commitsPerDay);
+    for (let i = 0; i < commitsCount; i++) {

Originally created by @djalilhebal on GitHub (Oct 23, 2023). Original GitHub issue: https://github.com/artiebits/fake-git-history/issues/42 First of all, nice utility. I just checked its source code and the loop's condition felt off. The end result will be fine (always < max commits per day), but it seems as if it's "changing its mind"... Imagine the following execution of `i < getRandomIntInclusive(0, 2)`: 1. First iteration: `0 < 1` 2. Second iteration: `1 < 2` 3. Third iteration: `2 < 0` A simple edit should make the intention clearer: ```diff - for (let i = 0; i < getRandomIntInclusive(...commitsPerDay); i++) { + const commitsCount = getRandomIntInclusive(...commitsPerDay); + for (let i = 0; i < commitsCount; i++) { ```

kerem closed this issue

2026-03-02 02:53:02 +03:00

kerem commented

2026-03-02 02:53:03 +03:00

@smcnary commented on GitHub (Oct 23, 2023):

Is the list on the my-history file supposed to be 1 entry or all the entries? Because on mine it's just the last entry in the block.

@smcnary commented on GitHub (Oct 23, 2023): Is the list on the my-history file supposed to be 1 entry or all the entries? Because on mine it's just the last entry in the block.

kerem commented

2026-03-02 02:53:03 +03:00

@artiebits commented on GitHub (Oct 23, 2023):

hi @djalilhebal,

a good catch, because it's actually a bug. indeed, the getRandomIntInclusive is being called on every iteration of the for loop. feel free to open a PR and fix it!

@artiebits commented on GitHub (Oct 23, 2023): hi @djalilhebal, a good catch, because it's actually a bug. indeed, the `getRandomIntInclusive` is being called on every iteration of the for loop. feel free to open a PR and fix it!

kerem commented

2026-03-02 02:53:03 +03:00

@artiebits commented on GitHub (Oct 29, 2023):

solved in #43

@artiebits commented on GitHub (Oct 29, 2023): solved in #43

kerem referenced this issue

2026-03-15 02:19:00 +03:00

[GH-ISSUE #17] Email address #68

Sign in to join this conversation.