-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add reusable ESLint workflow. #279
Conversation
Runs the eslint defined in the given package.json, configured by the associated eslint.config.mjs file.
if [ -n "${{ inputs.paths }}" ]; then | ||
echo "paths=$(echo '${{ inputs.paths }}' | tr '\n' ' ')" >> $GITHUB_OUTPUT | ||
else | ||
exit 1 | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: the if
statement not needed, as you have paths: required: true
in the inputs.
question: is this step even required at all?
env: | ||
PATHS: ${{ steps.parse-paths.outputs.paths }} | ||
run: | | ||
echo "$PATHS" | tr ' ' '\n' | xargs -I '{}' -n 1 sh -c 'ESLINT_USE_FLAT_CONFIG=true $1/node_modules/.bin/eslint -c $1/eslint.config.mjs "$1"' _ '{}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: I think making this step work directly with ${{ inputs.paths }}
might be worthwhile, maybe utilising bash's for
loop automatically splitting by space, tab, and newline...like
for path in $PATHS; do
do stuff
Removes complicated bash syntax
Now that it's using inputs.paths directly, don't need this parse step. Also, paths is required, so no need to check that paths have been given
No description provided.