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

Code refactor #269

Merged
merged 4 commits into from
Sep 22, 2022
Merged

Code refactor #269

merged 4 commits into from
Sep 22, 2022

Conversation

paescuj
Copy link
Collaborator

@paescuj paescuj commented Sep 21, 2022

Various optimizations:

  • Check all inputs before doing any API requests
  • Stricter check for doNotSkip input
  • Exclude current workflow run from list of all workflow runs
  • Get rid of redundant and unnecessary lines
  • Add some explaining comments
  • Fix bug with empty head_repository
  • Action might be a bit faster 🚀

Breaking changes:

  • Previously, on input and API request errors the action might have exited successfully but with outputs should_skip = false and reason = no_workflow_information. Now, the action will exit with an error instead.

    To make sure that subsequent jobs will not be skipped in such cases it's recommended to set continue-on-error: true in the skip-duplicate-actions job.
    Note: This has already been recommended in the past regardless of this change.

  • The structure of the skipped_by output (also in paths_filter) has slightly changed for the sake of consistency and alignment with GitHub API:

    • runId is now id
    • html_url is now htmlUrl
    • Full example:
      {
        "id": 3099801293,
        "runNumber": 930,
        "event": "pull_request",
        "treeHash": "179eceb71f2caf9f60b93d00818cc48ef41e057f",
        "commitHash": "9c6c638244dbc141ef254b676e80cb8beaa63be0",
        "status": "completed",
        "conclusion": "success",
        "htmlUrl": "https://github.com/fkirc/skip-duplicate-actions/actions/runs/3099801293",
        "branch": "code-refactor",
        "repo": "paescuj/skip-duplicate-actions",
        "workflowId": 2640563,
        "createdAt": "2022-09-21T17:21:55Z"
      }

Note: This refactoring also took place to be better able to address #264 (will follow in a separate pull request). Currently unsolvable...

@paescuj paescuj marked this pull request as draft September 21, 2022 14:39
@paescuj paescuj force-pushed the code-refactor branch 3 times, most recently from f46cc55 to d476e28 Compare September 21, 2022 15:54
@paescuj paescuj marked this pull request as ready for review September 21, 2022 15:56
@paescuj paescuj added the enhancement New feature or request label Sep 21, 2022
@paescuj paescuj requested a review from fkirc September 21, 2022 17:41
@fkirc
Copy link
Owner

fkirc commented Sep 21, 2022

The new code looks clean, I did not have the opportunity for detailed tests, but I think we should merge it and let the community test.

@paescuj
Copy link
Collaborator Author

paescuj commented Sep 22, 2022

The new code looks clean, I did not have the opportunity for detailed tests, but I think we should merge it and let the community test.

Thank you very much for your quick review!
I don't expect many users to be affected by those breaking changes, anyway...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants