Skip to content
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 option to select DIFF_HEAD for changed files comparsion #38

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

henry2423
Copy link

@henry2423 henry2423 commented Dec 4, 2020

There's an issue report on #23. The DIFF_BASE: ${{ github.base_ref }} may not work in some cases, for example, the pull_request checking. The reason for that is: For the pull_request, the actions/checkout will set the HEAD to the refs/remotes/pull/{PR_NUMBER}/merge, but the ${{ github.base_ref }} isn't set. So we have to fetch the ${{ github manually.base_ref }} (usually the master). The original diff comparison only runs on FETCH_HEAD and given base_ref. But in this case, the FETCH_HEAD will be the base_ref because we call the git fetch. Therefore, I set up a customizable HEAD and BASE parameters gives us more flexibility to deal with more jobs scenario, like the above case.

And for the backward compatibility, I didn't remove the old implement to ensure the others won't break.

name: CI

on: pull_request

jobs:
  PR-Check:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
      - name: Fetch base ref
        if: ${{ always() }}
        run: |
          git fetch --prune --no-tags --depth=1 origin +refs/heads/${{ github.base_ref }}:refs/heads/${{ github.base_ref }}
      - name: SwiftLint (Only files changed in the PR)
        if: ${{ always() }}
        uses: henry2423/action-swiftlint@Fix-PR-Lint
        with:
          args: --force-exclude --strict --config build-swiftlint.yml
        env:
           DIFF_BASE: ${{ github.base_ref }}
           DIFF_HEAD: HEAD

@richimf
Copy link

richimf commented Feb 18, 2021

This solution works for me, but I removed this line. What is this for?

with:
 args: --force-exclude --strict --config build-swiftlint.yml

@henry2423
Copy link
Author

henry2423 commented Feb 22, 2021

This solution works for me, but I removed this line. What is this for?

with:
 args: --force-exclude --strict --config build-swiftlint.yml

@richimf These configs are the options you can give to swiftlint lint. You can check it out with swiftlint help lint locally.

$swiftlint help lint

Print lint warnings and errors (default command)

[--path (string)]
	the path to the file or directory to lint

[--use-stdin]
	lint standard input

[--config (string)]
	the path to one or more SwiftLint configuration files, evaluated as a parent-child hierarchy

[--strict]
	upgrades warnings to serious violations (errors)

[--lenient]
	downgrades serious violations to warnings, warning threshold is disabled

[--force-exclude]
	exclude files in config `excluded` even if their paths are explicitly specified

[--use-alternative-excluding]
	alternative exclude paths algorithm for `excluded`.may speed up excluding for some cases

[--use-script-input-files]
	read SCRIPT_INPUT_FILE* environment variables as files

[--benchmark]
	save benchmarks to benchmark_files.txt and benchmark_rules.txt

[--reporter (string)]
	the reporter used to log errors and warnings

[--quiet]
	don't print status logs like 'Linting <file>' & 'Done linting'

[--cache-path (string)]
	the directory of the cache used when linting

[--no-cache]
	ignore cache when linting

[--enable-all-rules]
	run all rules, even opt-in and disabled ones, ignoring `only_rules`

[[""]]
	list of paths to the files or directories to lint

@jklein24
Copy link

jklein24 commented Apr 3, 2021

Any chance we can get this merged soon? It's a pretty major improvement and a ticket has been open for this issue for a pretty long time now.

jklein24 added a commit to mayk-it/action-swiftlint that referenced this pull request Apr 3, 2021
@CalebeRios
Copy link

Any news? This update is an important issue @norio-nomura

@hervenivon
Copy link

In case you don't already know,

I had issues with this https://github.com/norio-nomura/action-swiftlint. @sinoru's version https://github.com/sinoru/actions-swiftlint solved most of them, and the code is clearly maintained.

@kyzmitch
Copy link

kyzmitch commented Aug 2, 2023

In case you don't already know,

I had issues with this https://github.com/norio-nomura/action-swiftlint. @sinoru's version https://github.com/sinoru/actions-swiftlint solved most of them, and the code is clearly maintained.

I don't see that it is clearly maintained, there are no pull requests, it is not nice to promote own fork in the original repo

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.

6 participants