BLD: extract GH Actions check function to avoid duplication in code_checks.sh

[plammens]

This PR extracts a function if_gh_actions in ci/code_checks.sh to avoid command/arguments duplication. Originally part of #36386 (cherry-picked from 8611fe6); extracted to make the changes more modular (see the cross-linked comment).

• [N/A] closes #xxxx (xref CI/BLD: Restrict ci/code_checks.sh to tracked repo files #36386 (comment))
• [N.A] tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• [N/A] whatsnew entry

[jreback]

looks fine, cc @MarckK @datapythonista if any comments.

@plammens if you can merge master so we can get a green build

[plammens]

@plammens if you can merge master so we can get a green build

Done; rebased onto latest master.

[datapythonista]

I personally find this more complicated than needed. I agree having the if and duplicating the call should be simplified. But I think implementing a function makes things too complex.

What I would do is instead of:

    if [[ "$GITHUB_ACTIONS" == "true" ]]; then
        $BASE_DIR/scripts/validate_unwanted_patterns.py --validation-type="strings_to_concatenate" --format="##[error]{source_path}:{line_number}:{msg}" .
    else
        $BASE_DIR/scripts/validate_unwanted_patterns.py --validation-type="strings_to_concatenate" .
    fi

Simply have:

$BASE_DIR/scripts/validate_unwanted_patterns.py --validation-type="strings_to_concatenate" --format=$FORMAT

Then, there are several $FORMAT that will be the same, and there is already an if at the beginning of the file where the format for flake8 is already defined. Having an extra format variable defined there looks simpler than wrapping all calls with a bash function, xargs...

[plammens]

Simply have:

$BASE_DIR/scripts/validate_unwanted_patterns.py --validation-type="strings_to_concatenate" --format=$FORMAT

Then, there are several $FORMAT that will be the same, and there is already an if at the beginning of the file where the format for flake8 is already defined. Having an extra format variable defined there looks simpler than wrapping all calls with a bash function, xargs...

Yep this actually makes more sense. I think what happened is that I cherry picked this change, without giving it much thought, from #36386, where I think a function does make sense since there's already some xargs going on; but in isolation this would be a better solution.

On the other hand, if we consider the possibility that in the future other commands might depend on whether they're running on GH-actions (and in a less trivial way than just changing the format), it might be worthwhile to add the function.

[plammens]

@jreback @datapythonista Any more thoughts on this? Should I do what @datapythonista suggested (just extracting a FORMAT variable) in a new PR?

[jreback]

most of this is done in precommit anyhow

is this PR still needed?

[plammens]

is this PR still needed?

Just checked the code on master and it looks like this is no longer needed. Closing this.