Add Protection to, and clang-format Cycamore #644

dean-krueger · 2025-07-06T18:31:57Z

Summary of Changes

This (massive, sorry) PR does three things: adds //clang-format off and //clang-format on protection to all #pragma blocks in cycamore/src, cleans up some of the #pragma blocks/checks spelling and grammar in comments, and then runs clang-format on all the files in cycamore/src.

Related CEPs and Issues

This PR is related to:

Add Housekeeping to Check Code Style with clang-format cyclus#1674
Closes [Task] Add //clang-format off/on to protect all Cycamore Pragmas for clang-format #643

Associated Developers

@ahnaf-tahmid-chowdhury

Design Notes

This PR, which I expect I will be told should be three PRs because it sort of does three separate (large) things, but which I'm going to do my darnedest to sneak by as one, was submitted as one PR because I used ChatGPT to help with a lot of the manual addition of #pragma protection, and while it was doing that I asked it to run a spelling/grammar/general style check as well. Then, and most importantly, it would have been almost impossible to correctly check that I had done this correctly had I not been able to actually run clang-format on the code, at which point I figured I'd just submit everything since it's all tied together in this way.

Testing and Validation

The code was built locally on my machine. No additional tests were added.

Checklist

Read the Contributing to Cyclus guide.
Compile and run locally.
Add or update tests.
Document if needed.
Follow style guidelines.
Update the changelog.
Address all review comments.
Reviewers, please refer to the Cyclus Guide for Reviewers.

…matted some pragmas checking for column limits, spelling, and grammar.

dean-krueger · 2025-07-06T18:32:50Z

One day I'll remember the changelog before making a PR. One day.

dean-krueger · 2025-07-06T18:35:35Z

One final comment about the size of this PR: Because clang-format does so much I figured that instead of three +1500 / - 1496 line changes it might be better to do it all as one massive +3000 / -3000 line PR 😬

github-actions · 2025-07-06T18:36:17Z

Build Status Report - `0cb2ab9` - 2025-07-06 12:34:26 -0600

Build `FROM cyclus_20.04_apt/cyclus:latest`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_20.04_apt/cyclus:stable`

Cycamore: Failure ❌
Cymetric: Skipped due to upstream failure ⚠️

Build `FROM cyclus_20.04_conda/cyclus:latest`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_20.04_conda/cyclus:stable`

Cycamore: Failure ❌
Cymetric: Skipped due to upstream failure ⚠️

Build `FROM cyclus_22.04_apt/cyclus:latest`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_22.04_apt/cyclus:stable`

Cycamore: Failure ❌
Cymetric: Skipped due to upstream failure ⚠️

Build `FROM cyclus_22.04_conda/cyclus:latest`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_22.04_conda/cyclus:stable`

Cycamore: Failure ❌
Cymetric: Skipped due to upstream failure ⚠️

Build `FROM cyclus_24.04_apt/cyclus:latest`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_24.04_apt/cyclus:stable`

Cycamore: Failure ❌
Cymetric: Skipped due to upstream failure ⚠️

Build `FROM cyclus_24.04_conda/cyclus:latest`

Cycamore: Success ✅
Cymetric: Success ✅

Build `FROM cyclus_24.04_conda/cyclus:stable`

Cycamore: Failure ❌
Cymetric: Skipped due to upstream failure ⚠️

gonuke · 2025-07-06T19:09:07Z

I wonder if we can do this in cyclus/cyclus first and then get cyclus/cyclus#1674 implemented? It will need some clang-format "guards" as well for the archetypes that are defined there for testing

gonuke

I found one issue that may be problem when I started reviewing this - clang-format likes to reorder includes to be alphabetical, but we have an expectation (habit? need?) that we include cyclus.h before any other headers. Not sure if we can train clang-format to do something different, or if we have to make some other change to preserve the order.... or make the order less important.

gonuke · 2025-07-06T19:10:59Z

src/cycamore.h

 // These includes must come before others.
-#include "cyclus.h"
-#include "cycamore_version.h"


I wonder if we can force clang-format to not reorder these. I can't recall why these need to come first except that cyclus.h probably includes a whole bunch of stuff that we assume is included before the others are included.

I can just protect them with the comments like I do with the pragmas. That'd probably be the most straightforward way to do it.

Or this: https://stackoverflow.com/questions/60334299/clang-format-disable-ordering-includes

I think I found that same post a little while ago. The .clang-format for cyclus has SortIncludes: Never and I think that took care of things. If that's not already in the Cycamore one I'll for sure add it, though!

dean-krueger · 2025-07-07T15:17:17Z

I wonder if we can do this in cyclus/cyclus first and then get cyclus/cyclus#1674 implemented?

On it.

gonuke

I added SortIncludes in a suggestion comment, but I expect the whole code base will need to be reformatted from the original to preserve the original order... formatting in the branch will not sort them back

gonuke · 2025-08-13T01:35:25Z

.clang-format

+# Automatically detect whether a given fuction's arguments are one-per-line
+ExperimentalAutoDetectBinPacking: true
+# Use C++11 standard
+Standard: Cpp11


Suggested change

Standard: Cpp11

Standard: Cpp11

# prefer to keep includes sorted as we like

SortIncludes: Never

This reverts commit 1ccd928.

…s/Cyclus

dean-krueger · 2025-08-19T17:33:13Z

Okay two things have now been done here:

The PR now only adds the guard rails (//clang-format off/on) to cycamore, instead of that plus the format (should have put the includes back in the correct order)
The .clang-format document has been updated to be exactly like the one in cyclus/cyclus (added SortIncludes: Never)

gonuke · 2025-09-04T19:58:03Z

This appears to need a rebase...

gonuke · 2025-12-16T23:19:38Z

Pinging @dean-krueger because I think this only needs a rebase to be considered for merging...?

dean-krueger · 2025-12-22T18:07:31Z

On it!

dean-krueger added 3 commits July 5, 2025 14:11

added a clang format file to cycamore

52de381

added clang-format control protection around #pragma blocks and refor…

a826ec1

…matted some pragmas checking for column limits, spelling, and grammar.

ran clang-format on all files in cycamore/src

1ccd928

dean-krueger requested review from gonuke, katyhuff and munkm July 6, 2025 18:31

dean-krueger self-assigned this Jul 6, 2025

dean-krueger added Style Maintenance labels Jul 6, 2025

Changelog

0cb2ab9

gonuke requested changes Jul 6, 2025

View reviewed changes

gonuke requested changes Aug 13, 2025

View reviewed changes

dean-krueger added 2 commits August 19, 2025 12:27

Revert "ran clang-format on all files in cycamore/src"

f4d17f8

This reverts commit 1ccd928.

chagned .clang-format of cycamore to be identical to the one in Cyclu…

59c98cb

…s/Cyclus

dean-krueger requested a review from gonuke August 19, 2025 18:00

katyhuff removed their request for review September 4, 2025 15:10

dean-krueger added the DK Side Project label Nov 24, 2025

dean-krueger added this to the DK Stranded milestone Nov 24, 2025

Add Protection to, and clang-format Cycamore #644

Are you sure you want to change the base?

Add Protection to, and clang-format Cycamore #644

Uh oh!

Conversation

dean-krueger commented Jul 6, 2025

Summary of Changes

Related CEPs and Issues

Associated Developers

Design Notes

Testing and Validation

Checklist

Uh oh!

dean-krueger commented Jul 6, 2025

Uh oh!

dean-krueger commented Jul 6, 2025

Uh oh!

github-actions bot commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Build Status Report - 0cb2ab9 - 2025-07-06 12:34:26 -0600

Build FROM cyclus_20.04_apt/cyclus:latest

Build FROM cyclus_20.04_apt/cyclus:stable

Build FROM cyclus_20.04_conda/cyclus:latest

Build FROM cyclus_20.04_conda/cyclus:stable

Build FROM cyclus_22.04_apt/cyclus:latest

Build FROM cyclus_22.04_apt/cyclus:stable

Build FROM cyclus_22.04_conda/cyclus:latest

Build FROM cyclus_22.04_conda/cyclus:stable

Build FROM cyclus_24.04_apt/cyclus:latest

Build FROM cyclus_24.04_apt/cyclus:stable

Build FROM cyclus_24.04_conda/cyclus:latest

Build FROM cyclus_24.04_conda/cyclus:stable

Uh oh!

gonuke commented Jul 6, 2025

Uh oh!

gonuke left a comment

Choose a reason for hiding this comment

Uh oh!

gonuke Jul 6, 2025

Choose a reason for hiding this comment

Uh oh!

dean-krueger Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

gonuke Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

dean-krueger Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

dean-krueger commented Jul 7, 2025

Uh oh!

gonuke left a comment

Choose a reason for hiding this comment

Uh oh!

gonuke Aug 13, 2025

Choose a reason for hiding this comment

Uh oh!

dean-krueger commented Aug 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gonuke commented Sep 4, 2025

Uh oh!

gonuke commented Dec 16, 2025

Uh oh!

dean-krueger commented Dec 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Jul 6, 2025 •

edited

Loading

Build Status Report - `0cb2ab9` - 2025-07-06 12:34:26 -0600

Build `FROM cyclus_20.04_apt/cyclus:latest`

Build `FROM cyclus_20.04_apt/cyclus:stable`

Build `FROM cyclus_20.04_conda/cyclus:latest`

Build `FROM cyclus_20.04_conda/cyclus:stable`

Build `FROM cyclus_22.04_apt/cyclus:latest`

Build `FROM cyclus_22.04_apt/cyclus:stable`

Build `FROM cyclus_22.04_conda/cyclus:latest`

Build `FROM cyclus_22.04_conda/cyclus:stable`

Build `FROM cyclus_24.04_apt/cyclus:latest`

Build `FROM cyclus_24.04_apt/cyclus:stable`

Build `FROM cyclus_24.04_conda/cyclus:latest`

Build `FROM cyclus_24.04_conda/cyclus:stable`

dean-krueger commented Aug 19, 2025 •

edited

Loading