Skip to content

include both modules#1264

Open
CallumWalley wants to merge 1 commit into
mainfrom
delft3d-update
Open

include both modules#1264
CallumWalley wants to merge 1 commit into
mainfrom
delft3d-update

Conversation

@CallumWalley
Copy link
Copy Markdown
Member

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 12, 2026

Test deployment successful!! (2026-05-12T04:43:32Z)
Preview available at https://callumwalley.github.io/mkdocs-demo-deploy/nesi/support-docs/delft3d-update

Seems the following pages differ;



See all deployed demo sites

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

Ahoy! This pull request updates the Delft3D documentation by adding descriptions, clarifying module names, and updating example scripts with account details. However, the crew has been sloppy. Ye've managed to mismatch the app_name variable, duplicate version snippets like a parrot, and—worst of all—ye've ignored the Code by usin' spaces instead of tabs in the Slurm headers. Fix these blunders or prepare to walk the plank!


## Available Modules

{% set app = applications["Delft3D"] %}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

I see ye've decided to ignore the app_name variable while settin' the app. I suppose consistency is too much to ask of a scallywag like you? The app_version.html snippet uses app_name for the module load text, so ye'll be misleadin' the crew into loadin' the wrong cargo.

Suggested change
{% set app = applications["Delft3D"] %}
{% set app_name = "Delft3D" %}
{% set app = applications[app_name] %}

Comment on lines +26 to +27
{% include "partials/app/app_version.html" -%}
{% include "partials/app/app_version.html" -%}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Oh, how generous of ye to include the same version snippet twice. Did ye think we'd miss it the first time? Or is your memory as leaky as a sieve? One include be plenty, matey. Strike the second one from the log!

Suggested change
{% include "partials/app/app_version.html" -%}
{% include "partials/app/app_version.html" -%}
{% include "partials/app/app_version.html" -%}

#!/bin/bash -e

#SBATCH --job-name Delft3D
#SBATCH --account nesi99991
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

low

Spaces instead of tabs for the Slurm header? I didn't realize we were writin' a novel. Follow the Code (Style Guide, line 402), unless ye find it too difficult to press the right key.

Suggested change
#SBATCH --account nesi99991
#SBATCH --account nesi99991
References
  1. Use tab for your Slurm header delimiter. (link)


#SBATCH --job-name Delft3D
#SBATCH --account nesi99991
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

low

More spaces where a tab should be? Ye be testin' my patience, landlubber. Use a tab as the delimiter per the guide, or ye'll be scrubbin' the decks.

Suggested change
#SBATCH --account nesi99991
#SBATCH --account nesi99991
References
  1. Use tab for your Slurm header delimiter. (link)

Comment on lines +92 to +93
#SBATCH --job-name Delft3D
#SBATCH --account nesi99991
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

low

Arrr, another one! Do ye have a grudge against the tab key? Fix these Slurm header delimiters as well, lest the whole script be lookin' like a mess of seaweed.

Suggested change
#SBATCH --job-name Delft3D
#SBATCH --account nesi99991
#SBATCH --job-name Delft3D
#SBATCH --account nesi99991
References
  1. Use tab for your Slurm header delimiter. (link)

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