Skip to content

Conversation

@Agraphie
Copy link
Contributor

The underlying Squawk CLI features many more parameters than the current action. This PR introduces multiple CI relevant parameters so it's easier to configure them when using this action.

It also simplifies the pattern and files usage into one Squawk call instead of two.

Agraphie added 2 commits June 10, 2024 15:04
The base Squawk CLI defines some parameters which are not exposed via the action. This adds the CI relevant ones.
This removes the need for two Squawk calls.
@sbdchd sbdchd requested a review from chdsbd June 10, 2024 15:14
@chdsbd
Copy link
Collaborator

chdsbd commented Jun 10, 2024

Here's a PR with the old version: recipeyak/recipeyak#1485

Here's a PR with the new version: recipeyak/recipeyak#1484

I think we need to adjust how we find files

@chdsbd
Copy link
Collaborator

chdsbd commented Jun 11, 2024

Ohhhh, I think there was a regression introduced in a recent commit

@chdsbd
Copy link
Collaborator

chdsbd commented Jun 11, 2024

I think we may want to revert 4828f43

We can add proper pattern support inside the squawk project. That way we can fix this issue

@Agraphie Agraphie force-pushed the feature/some-squawk-params branch from da53618 to ae25674 Compare June 11, 2024 07:15
@Agraphie
Copy link
Contributor Author

Thanks for the tests. The problem is the leading or non-leading ./. I've changed the command to account for both and it tested it with a repo. Can you recheck please? The drawback of the previous compgen solution is that it doesn't really do multi-path glob pattern, but only for one path.

Natively supporting a pattern i.e. a list of files including patterns would be nice though, then we can save one parameter here

@chdsbd
Copy link
Collaborator

chdsbd commented Jun 11, 2024

@Agraphie Looks good! Thanks for the quick update!

@chdsbd chdsbd merged commit 9169df9 into sbdchd:main Jun 11, 2024
@Agraphie Agraphie deleted the feature/some-squawk-params branch June 11, 2024 15:05
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