Skip to content

use non-empty array qualifiers - #328

Merged
a-frantz merged 3 commits into
mainfrom
non-empty
Aug 19, 2026
Merged

use non-empty array qualifiers#328
a-frantz merged 3 commits into
mainfrom
non-empty

Conversation

@a-frantz

@a-frantz a-frantz commented Aug 19, 2026

Copy link
Copy Markdown
Member

We attempted and reverted using non-empty qualifies on arrays in an earlier PR (#240) due to friction caused by Sprocket analysis and odd type coercion. That was solved a long time ago and now that friction is gone. It was trivial to make this type update, which was a very pleasant surprise.

also includes drive-by typo and test definition fixes

Describe the problem or feature in addition to a link to the issues.

Before submitting this PR, please make sure:

  • You have added a few sentences describing the PR here.
  • The code passes all CI tests without any errors or warnings.
  • You have added tests (when appropriate).
  • You have added an entry in any relevant CHANGELOGs (when appropriate).
  • If you have made any changes to the scripts/ or docker/ directories, please ensure any image versions have been incremented accordingly!
  • You have updated the README or other documentation to account for these changes (when appropriate).

@stjudecloud-cloudy

stjudecloud-cloudy commented Aug 19, 2026

Copy link
Copy Markdown

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@adthrasher adthrasher changed the title use non-emtpy array qualifiers use non-empty array qualifiers Aug 19, 2026
@a-frantz a-frantz self-assigned this Aug 19, 2026
adthrasher
adthrasher previously approved these changes Aug 19, 2026
Comment thread tools/test/samtools.yaml
outputs:
sampled_bam:
Defined: true
- Defined: true

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.

Have these been quietly broken or did something change in sprocket?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

they've been silently broken I believe. Alex has done some recent work on improving the diagnostics emitted from broken test definitions. I think the initial implementation WARNed about them, but they were easy to miss in the giant stream of logs. Alex made them errors semi-recently.

cc @Serial-ATA

Comment thread tools/samtools.wdl
@a-frantz
a-frantz marked this pull request as ready for review August 19, 2026 15:05
@adthrasher adthrasher mentioned this pull request Aug 19, 2026
6 tasks
@a-frantz
a-frantz requested a review from adthrasher August 19, 2026 18:39
adthrasher added a commit that referenced this pull request Aug 19, 2026
Adds spellcheck to the CI. Some of the changes duplicate those in #328.

Before submitting this PR, please make sure:

- [ ] You have added a few sentences describing the PR here.
- [ ] The code passes all CI tests without any errors or warnings.
- [ ] You have added tests (when appropriate).
- [ ] You have added an entry in any relevant CHANGELOGs (when
appropriate).
- [ ] If you have made any changes to the `scripts/` or `docker/`
directories, please ensure any image versions have been incremented
accordingly!
- [ ] You have updated the README or other documentation to account for
these changes (when appropriate).
Comment thread tools/test/ngsderive.yaml
@a-frantz
a-frantz merged commit 93e4e39 into main Aug 19, 2026
63 checks passed
@a-frantz
a-frantz deleted the non-empty branch August 19, 2026 20:02
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.

3 participants