Skip to content

Fix some typos #43

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

Merged
JeffGarland merged 3 commits into from
Jul 7, 2025
Merged

Fix some typos #43

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
84eb2f1
use llvm-toolchain.cmake
ClausKlein Jul 5, 2025
80a9146
Fix some typos with codespell
ClausKlein Jul 5, 2025
b21d19f
revert to cpp20 CMakePresets.json
JeffGarland Jul 7, 2025
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
6 changes: 3 additions & 3 deletions .devcontainer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ RUN wget -O - https://apt.kitware.com/keys/kitware-archive-latest.asc 2>/dev/nul
RUN echo 'deb [signed-by=/usr/share/keyrings/kitware-archive-keyring.gpg] https://apt.kitware.com/ubuntu/ noble main' | sudo tee /etc/apt/sources.list.d/kitware.list >/dev/null
RUN sudo apt-get update && sudo apt-get install -y cmake

# Install pre-commit
RUN sudo apt-get install -y pipx && pipx install pre-commit
# Install pre-commit and ninja
RUN sudo apt-get install -y pipx && pipx install pre-commit && pipx install ninja

# Newer gcc/ llvm is needed to avoid ASAN Stalling
# See: https://github.com/google/sanitizers/issues/1614
# Minimal vesion: clang-18.1.3, gcc-13.2
# Minimal version: clang-18.1.3, gcc-13.2
RUN sudo apt-get install -y gcc-14
10 changes: 5 additions & 5 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ repos:

# CMake linting and formatting
- repo: https://github.com/BlankSpruce/gersemi
rev: 0.15.1
rev: 0.20.0
hooks:
- id: gersemi
name: CMake linting

# Markdown linting
# Config file: .markdownlint.yaml
- repo: https://github.com/igorshubovych/markdownlint-cli
rev: v0.42.0
hooks:
- id: markdownlint
# - repo: https://github.com/igorshubovych/markdownlint-cli
# rev: v0.42.0
# hooks:
# - id: markdownlint
7 changes: 4 additions & 3 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

cmake_minimum_required(VERSION 3.28)
cmake_minimum_required(VERSION 3.28...4.1)
Copy link
Member

@JeffGarland JeffGarland Jul 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

edit: suggestion-removed...

cmake_minimum_required(VERSION 3.28)

I believe on the advice of @nickelpro we are explicitly not putting in ranges

Copy link
Contributor Author

@ClausKlein ClausKlein Jul 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

if on ci is only cmake v3.28, I need this to test import std;

Copy link
Member

@nickelpro nickelpro Jul 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm in favor of version ranges.

There was confusion over what a cmake_minimum_required(VERSION MIN...MAX) meant among some beman maintainers, ie, that it forbid the project from building with a version >MAX, which led to some discussion of avoiding ranges.

This is not the behavior of a range. Instead it offers to CMake a range of behaviors it may emulate. cmake_minimum_required(VERSION 3.28) requires that CMake offer bug-for-bug compatibility with CMake 3.28. cmake_minimum_required(VERSION 3.28...4.1) allows CMake to choose any version in this range to target.

If a maintainer is comfortable with offering a range, we should allow them to do so.

Copy link
Member

@JeffGarland JeffGarland Jul 7, 2025

Choose a reason for hiding this comment

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

thanks for the clarification -- that's actually good news :)


project(
beman.scope
Expand All @@ -16,7 +16,7 @@ if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER_VERSION VERSIO
set(CMAKE_CXX_SCAN_FOR_MODULES 1)
elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 15)
set(CMAKE_CXX_SCAN_FOR_MODULES 1)
elseif()
else()
set(CMAKE_CXX_SCAN_FOR_MODULES 0)
endif()

Expand Down Expand Up @@ -116,7 +116,8 @@ if(BEMAN_SCOPE_INSTALL_CONFIG_FILE_PACKAGE)
EXPORT beman.scope-targets
DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/beman.scope
NAMESPACE beman::
CXX_MODULES_DIRECTORY cxx-modules
CXX_MODULES_DIRECTORY
cxx-modules
COMPONENT beman.scope
)
endif()
Expand Down
4 changes: 2 additions & 2 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"_debug-base"
],
"cacheVariables": {
"CMAKE_TOOLCHAIN_FILE": "cmake/appleclang-toolchain.cmake"
"CMAKE_TOOLCHAIN_FILE": "cmake/llvm-toolchain.cmake"
}
},
{
Expand All @@ -66,7 +66,7 @@
"_release-base"
],
"cacheVariables": {
"CMAKE_TOOLCHAIN_FILE": "cmake/appleclang-toolchain.cmake"
"CMAKE_TOOLCHAIN_FILE": "cmake/llvm-toolchain.cmake"
}
},
{
Expand Down
8 changes: 7 additions & 1 deletion tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ FetchContent_MakeAvailable(Catch2)

# module tests will only compile with gcc15 or clang20 and above
if(CMAKE_CXX_SCAN_FOR_MODULES)
set(ALL_TESTNAMES scope_success scope_exit scope_fail unique_resource module)
set(ALL_TESTNAMES
scope_success
scope_exit
scope_fail
unique_resource
module
)
else()
set(ALL_TESTNAMES scope_success scope_exit scope_fail unique_resource)
endif()
Expand Down