Skip to content

Add build-test-pkgs and extra-pkgs inputs#20

Merged
stertooy merged 8 commits intogap-actions:mainfrom
stertooy:testpkgs
Mar 24, 2026
Merged

Add build-test-pkgs and extra-pkgs inputs#20
stertooy merged 8 commits intogap-actions:mainfrom
stertooy:testpkgs

Conversation

@stertooy
Copy link
Copy Markdown
Contributor

The first input is as suggested in a Slack conversation.

The second input, well, I'm not sure if it's needed - ideally everything would be covered by the other inputs, But it probably doesn't hurt to have either.

@stertooy stertooy marked this pull request as draft March 20, 2026 15:57
@stertooy
Copy link
Copy Markdown
Contributor Author

Converting this to draft. Ideally this will be built on top of #22.

@stertooy stertooy marked this pull request as ready for review March 21, 2026 11:25
action.yml Outdated
Comment on lines +152 to +163
if ( IsBound( info.Dependencies.TestPackages ) and (
"${{ inputs.build-test-pkgs }}" = "recursive" or
( "${{ inputs.build-test-pkgs }}" = "true" and depth = 1 )
)) then
for pkg in info.Dependencies.TestPackages do
name := LowercaseString( pkg[1] );
Print( " - is tested using package '", name, "'\n" );
if not name in done then
AddSet( todo, name );
fi;
od;
fi;
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 code is now repeated for the 4th time. How about factoring the bulk out into a helper function AddDeps (better name welcome), which might be called as, say,

Suggested change
if ( IsBound( info.Dependencies.TestPackages ) and (
"${{ inputs.build-test-pkgs }}" = "recursive" or
( "${{ inputs.build-test-pkgs }}" = "true" and depth = 1 )
)) then
for pkg in info.Dependencies.TestPackages do
name := LowercaseString( pkg[1] );
Print( " - is tested using package '", name, "'\n" );
if not name in done then
AddSet( todo, name );
fi;
od;
fi;
AddDeps( "TestPackages", "${{ inputs.build-test-pkgs }}", "is tested using package" );

and that helper might look roughly like this (untested, and indentation ought to be fixed):

Suggested change
if ( IsBound( info.Dependencies.TestPackages ) and (
"${{ inputs.build-test-pkgs }}" = "recursive" or
( "${{ inputs.build-test-pkgs }}" = "true" and depth = 1 )
)) then
for pkg in info.Dependencies.TestPackages do
name := LowercaseString( pkg[1] );
Print( " - is tested using package '", name, "'\n" );
if not name in done then
AddSet( todo, name );
fi;
od;
fi;
AddDeps := function(field, mode, desc)
if ( IsBound( info.Dependencies.(field) ) and (
mode = "recursive" or
( mode = "true" and depth = 1 )
)) then
for pkg in info.Dependencies.(field) do
name := LowercaseString( pkg[1] );
Print( " - ", desc, " '", name, "'\n" );
if not name in done then
AddSet( todo, name );
fi;
od;
fi;
end;

@stertooy stertooy merged commit 9a2931f into gap-actions:main Mar 24, 2026
23 checks passed
@stertooy stertooy deleted the testpkgs branch March 24, 2026 08:20
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