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

feat: update to CXX standard 17 and add CMakeLists file to directories without them #2746

Merged
merged 50 commits into from
Nov 4, 2024

Conversation

realstealthninja
Copy link
Collaborator

@realstealthninja realstealthninja commented Oct 1, 2024

Description of Change

CMakeLists.txt changes

Some directories lack CMakeLists Thus giving any algorithms under those directories

  • dynamic_programming
  • greedy_algorithms
  • range_queries
  • operations_on_datastructures

cmake specific changes

  • Incremented CXX standard (see reasoning below)
  • Removed old commented cmake statements
  • Added warnings
  • Made msvc be permissive

So this pr focuses on the the inclusion of the CMakeLists file and fixing compilation problems
associated with the inclusion of the directories

File renames

some files had to be renamed due to file name conflicts with pre-existing files. I would love feedback on the names. These algorithms are duplicates with alot of history associated with them removing them seemed like not my decision to make.

Code changes

Some older code was updated very slightly just enough to make them compile as the scope of this pr is not part of #2456

The updated code include;

  • Variable length arrays are fixed by using either vectors or dynamic arrays
  • linting done on files edited

Standard updated

This is the part of the pr where I explain my thought process behind this update in such a minor pr.

here are some of my arguments:

  • Some algorithms in the repository use newer features from newer standards
  • The Readme mentions compatibility with esp32 & arm cortex however they already use newer standards
  • C++11 is 13 years old
  • People learn using C++17 it only makes sense for us to provide algorithms showcasing newer features

Checklist

  • Added description of change
  • Added file name matches File name guidelines
  • Added tests and example, test must pass
  • Added documentation so that the program is self-explanatory and educational - Doxygen guidelines
  • Relevant documentation/comments is changed or added
  • 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:

@realstealthninja
Copy link
Collaborator Author

#2717 is a prerequisite to this pr.

@realstealthninja realstealthninja marked this pull request as ready for review October 4, 2024 15:39
@realstealthninja realstealthninja changed the title chore: Update CXX standard to 17 and add CMakeLists.txt to directories without them feat: Update CXX standard and add CMakeLists file to directories without them Oct 4, 2024
@realstealthninja realstealthninja changed the title feat: Update CXX standard and add CMakeLists file to directories without them feat: Update to CXX standard 17 and add CMakeLists file to directories without them Oct 4, 2024
@realstealthninja realstealthninja added dont-close This issue/pull request shouldn't be closed awaiting review pull request is waiting to be reviewed labels Oct 4, 2024
@realstealthninja realstealthninja changed the title feat: Update to CXX standard 17 and add CMakeLists file to directories without them feat: update to CXX standard 17 and add CMakeLists file to directories without them Oct 4, 2024
This was referenced Oct 8, 2024
@realstealthninja realstealthninja merged commit 0d766b0 into TheAlgorithms:master Nov 4, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review pull request is waiting to be reviewed dont-close This issue/pull request shouldn't be closed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant