Skip to content

Conversation

@bartgol
Copy link
Contributor

@bartgol bartgol commented Sep 4, 2025

Allows us to pick the version without relying on what is available on the runner.

[BFB]


Allows us to pick the version without relying
on what is available on the runner.
@bartgol bartgol requested a review from mjschmidt271 September 4, 2025 23:38
@bartgol bartgol self-assigned this Sep 4, 2025
@bartgol bartgol added BFB PR leaves answers BFB workflows labels Sep 4, 2025
Copy link
Contributor

@mahf708 mahf708 left a comment

Choose a reason for hiding this comment

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

not the recommended way to deal with conda tools... see "install python deps" here for a better example:

- name: Install Python deps
uses: mamba-org/setup-micromamba@v2
with:
init-shell: bash
environment-name: docs
create-args: >-
python=3.12
mkdocs-material
pymdown-extensions
mkdocs-monorepo-plugin
mdutils
mkdocs-bibtex==2.18.0
marp-cli

note how in that workflow you neve have to activate the env ... (it's all done for you)

@mahf708
Copy link
Contributor

mahf708 commented Sep 5, 2025

also pls add workflow path itself to this section

on:
  pull_request:
    branches: ["master"]
    paths:
      - 'components/eamxx/**/*.cpp'
      - 'components/eamxx/**/*.hpp'
+     - .github/workflows/eamxx-gh-clang-format.yml

@bartgol
Copy link
Contributor Author

bartgol commented Sep 5, 2025

The micromamba version worked great, with no extra verbosity in the shell. See this run on my fork.

Copy link
Contributor

@mjschmidt271 mjschmidt271 left a comment

Choose a reason for hiding this comment

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

Looks great--nice work!

@mahf708
Copy link
Contributor

mahf708 commented Sep 5, 2025

you may need something like to make it work better

  Build-and-Deploy-docs:
    if: ${{ github.repository == 'E3SM-Project/E3SM' }}
    defaults:
      run:
        shell: bash -leo pipefail {0}

and also, you can drop the "conda activate" lines later

If workflow file is modified but no eamxx src files are,
clang-format won't format any file, but we will at least
ensure that the worfklow will still run correctly
@bartgol
Copy link
Contributor Author

bartgol commented Sep 5, 2025

you may need something like to make it work better

  Build-and-Deploy-docs:
    if: ${{ github.repository == 'E3SM-Project/E3SM' }}
    defaults:
      run:
        shell: bash -leo pipefail {0}

and also, you can drop the "conda activate" lines later

Good call! The workflow was actually still running clang-format-18 (from the OS!). I am testing this on my fork now.

@bartgol bartgol force-pushed the bartgol/workflows/eamxx-clang-format-with-conda branch from 30b3259 to 7efd203 Compare September 5, 2025 17:35
- Run on forks as well
- Print version before formatting files
@bartgol
Copy link
Contributor Author

bartgol commented Sep 5, 2025

Ok, seems to run as expected (see here).

@bartgol bartgol requested a review from mjschmidt271 September 5, 2025 17:50
@mahf708
Copy link
Contributor

mahf708 commented Sep 5, 2025

I defer to @mjschmidt271 on the version desired

@bartgol
Copy link
Contributor Author

bartgol commented Sep 5, 2025

FYI, I picked 20.1.8 because...it ran! The default version we have in master (18, from the runner OS) was getting a segfault when using the new settings in #7666. Since we don't have any real reason to pick one version or another (except that it must be >=17, so that those new options will work), I just used whatever my 1st attempt with conda install gave me, which happened to be 20.1.8 and ran correctly.

If you guys have any reason to prefer other versions, I can change (or we can update in the future).

@mjschmidt271
Copy link
Contributor

v20.1.8 is just fine with me. I don't really have any great attachment to a particular version as long as it works and remains consistent for the long-medium term

@bartgol bartgol merged commit a91a2d5 into master Sep 5, 2025
3 checks passed
@bartgol bartgol deleted the bartgol/workflows/eamxx-clang-format-with-conda branch September 5, 2025 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BFB PR leaves answers BFB workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants