Skip to content

✨ Quality: Acknowledge that the Sieve of Eratosthenes implementation is "unfaithful" #1154

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.

Sign up for GitHub

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

Open
andoan16 wants to merge 3 commits into
base: master
Choose a base branch
from
+14 −836
Open

✨ Quality: Acknowledge that the Sieve of Eratosthenes implementation is "unfaithful" #1154

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c3640e
refactor: acknowledge that the sieve of eratosthenes implementation i…
andoan16 Mar 28, 2026
3c4710e
fix: address review feedback — Please reformat it.
andoan16 Mar 29, 2026
40d6d95
fix: address review feedback — Please reformat it.
andoan16 Mar 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions xml/chapter3/section5/subsection2.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,7 @@ const fibs = fibgen(0, 1);
numbers, using a method known as the
<INDEX>prime number(s)<SUBINDEX>Eratosthenes<APOS/>s sieve for</SUBINDEX></INDEX>
<INDEX>sieve of Eratosthenes</INDEX>
<FOOTNOTE>This is not actually the Sieve of Eratosthenes, but rather a trial division sieve. The genuine Sieve of Eratosthenes marks off multiples of each prime starting from the prime's square, whereas this algorithm tests each candidate number by dividing it by all previously found primes. See Melissa E. O'Neill, <QUOTE>The Genuine Sieve of Eratosthenes,</QUOTE> <EM>Journal of Functional Programming</EM>, 2009.</FOOTNOTE>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The placement of this footnote is unconventional. For better readability, a footnote should typically be placed immediately after the term it clarifies, not before.

I recommend moving this <FOOTNOTE> to be after </EM> on line 236. The structure would then look like this:

...
    <EM>sieve of
    Eratosthenes</EM><FOOTNOTE>This is not actually the Sieve of Eratosthenes...</FOOTNOTE>.<FOOTNOTE>Eratosthenes, ...</FOOTNOTE>
...

This will correctly associate the footnote with the term "sieve of Eratosthenes".

Copy link
Copy Markdown
Member

@RichDom2185 RichDom2185 Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reformat it.

Copy link
Copy Markdown
Author

@andoan16 andoan16 Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Addressed this feedback in commit fix: address review feedback — Please reformat it.. Thanks for the review!

<EM>sieve of
Eratosthenes</EM>.<FOOTNOTE>Eratosthenes,
<INDEX>Eratosthenes</INDEX>
Expand Down