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

fix: rewrite prime numbers using linear sieve #1810

Merged
merged 6 commits into from
Oct 31, 2021
Merged

fix: rewrite prime numbers using linear sieve #1810

merged 6 commits into from
Oct 31, 2021

Conversation

nella17
Copy link
Contributor

@nella17 nella17 commented Oct 29, 2021

Description of Change

Rewrite math/prime_numbers.cpp using linear sieve.

Checklist

  • Added description of change
  • Added documentation so that the program is self-explanatory and educational - Doxygen guidelines
  • PR title follows semantic commit guidelines
  • Search previous suggestions before making a new one, as yours may be a duplicate.
  • I acknowledge that all my contributions will be made under the project's license.

Notes: made change in math/prime_numbers.cpp

@Panquesito7 Panquesito7 added the Improvement improvement in previously written codes label Oct 29, 2021
@Panquesito7 Panquesito7 self-requested a review October 29, 2021 04:09
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Thank you for your contribution! 😄👍
Do you want to take this opportunity to update this file up to the standards? I'll guide you with reviews. 🙂

@Panquesito7 Panquesito7 added the approved Approved; waiting for merge label Oct 29, 2021
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

Oh, I just saw this 😅 could you please fix the errors mentioned here?
Let us know if you need any help here or in our Discord server! 🙂

@Panquesito7 Panquesito7 added requested changes changes required and removed approved Approved; waiting for merge labels Oct 29, 2021
@nella17
Copy link
Contributor Author

nella17 commented Oct 29, 2021

Panquesito7
Panquesito7 previously approved these changes Oct 29, 2021
@Panquesito7 Panquesito7 added approved Approved; waiting for merge and removed requested changes changes required labels Oct 29, 2021
aminoxix
aminoxix previously approved these changes Oct 30, 2021
Copy link
Member

@ayaankhan98 ayaankhan98 left a comment

Choose a reason for hiding this comment

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

test.cpp: In function ‘std::vector<int> primes(int)’:
test.cpp:15:26: error: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘int’ [-Werror=sign-compare]
   15 |     for (size_t i = 2; i <= max; i++) {
      |                        ~~^~~~~~
test.cpp:20:19: error: comparison of integer expressions of different signedness: ‘size_t’ {aka ‘long unsigned int’} and ‘int’ [-Werror=sign-compare]
   20 |             if (k > max)
      |                 ~~^~~~~
cc1plus: all warnings being treated as errors

@nella17 nella17 dismissed stale reviews from aminoxix and Panquesito7 via 6ddff0f October 30, 2021 10:39
Copy link
Member

@Panquesito7 Panquesito7 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 Please review again, @ayaankhan98.

Copy link
Member

@aminoxix aminoxix left a comment

Choose a reason for hiding this comment

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

Looks good!

@Panquesito7 Panquesito7 merged commit 1f6c39b into TheAlgorithms:master Oct 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved; waiting for merge Improvement improvement in previously written codes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants