Skip to content

Simplify Housekeeping Process for DAGMC#943

Merged
gonuke merged 11 commits into
svalinn:developfrom
ahnaf-tahmid-chowdhury:housekeeping
Feb 22, 2024
Merged

Simplify Housekeeping Process for DAGMC#943
gonuke merged 11 commits into
svalinn:developfrom
ahnaf-tahmid-chowdhury:housekeeping

Conversation

@ahnaf-tahmid-chowdhury

Copy link
Copy Markdown
Member

Description

This PR simplifies the housekeeping process for DAGMC by removing the dependency on a Docker container and installing clang-format directly through the workflow.

Related Issues

Housekeeping of DAGMC is very complicated now. And it seems in the docker file, we are just installing clang-format through apt. Also, I noticed the docker publish file is changed and there is no way to rebuild a new docker image for housekeeping.

Changes

  • Removed the use of a Docker container for housekeeping.
  • Installed clang-format directly through the workflow.

@ahnaf-tahmid-chowdhury ahnaf-tahmid-chowdhury changed the title remove using container Simplify Housekeeping Process for DAGMC Feb 20, 2024
@gonuke

gonuke commented Feb 20, 2024

Copy link
Copy Markdown
Member

perhaps we should update these two files to match what clang-format is generating as part of this PR so that it passes tests

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks good - I thought I had done this already :) but maybe it's lost in my local branches somewhere.

We can perhaps take care of the two formatting inconsistencies in the current code that are causing this to fail right now. I wonder if they are due to slight changes in clang-format's behavior since the docker images was generated.

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One small request

Comment thread src/geant4/app/include/ExN01UserScoreWriter.hh Outdated
gonuke
gonuke previously approved these changes Feb 21, 2024

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for streaming in this @ahnaf-tahmid-chowdhury

@gonuke

gonuke commented Feb 21, 2024

Copy link
Copy Markdown
Member

This now has a conflict in the CHANGELOG.rst

@gonuke

gonuke commented Feb 22, 2024

Copy link
Copy Markdown
Member

It looks like there is an update in CMake version in the new Windows runner... I'm not sure yet if/why that is causing a problem.

@gonuke

gonuke commented Feb 22, 2024

Copy link
Copy Markdown
Member

We may need to provide more information to our CMake config for MOAB. It may be true that MOAB should improve their custom FindEigen macro, but providing EIGEN3_DIR may be easier

@gonuke gonuke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for cleaning this up @ahnaf-tahmid-chowdhury

@gonuke gonuke merged commit ab4db53 into svalinn:develop Feb 22, 2024
@ahnaf-tahmid-chowdhury ahnaf-tahmid-chowdhury deleted the housekeeping branch February 22, 2024 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants