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

Improve check-theme-changes job #16

Closed
wants to merge 1 commit into from
Closed

Improve check-theme-changes job #16

wants to merge 1 commit into from

Conversation

szepeviktor
Copy link

@szepeviktor szepeviktor commented Oct 10, 2023

  1. Use actions/checkout action to fetch
  2. Use git diff --quiet to decide whether there were changes

📜 https://git-scm.com/docs/git-diff

--prune is added by default
https://github.com/actions/checkout/blob/8ade135a41bc03ea155e62e844d188df1ea18608/src/git-command-manager.ts#L257

@codepuncher
Copy link
Member

Hi @szepeviktor,

Thanks for your PR!
It's appreciated; looks a better approach.

I'm not sure there is an easy way to test this before merging due to nature of refs with GitHub Actions.
Do you have any idea/suggestion?

@szepeviktor
Copy link
Author

I'm not sure there is an easy way to test this before merging

Yes, there is. You can test it in a fork.

@codepuncher
Copy link
Member

Hi @szepeviktor
Thanks again for the PR.

I have just tested it in the https://github.com/ItinerisLtd/.github/compare/test-patch branch so I can use it in one of our projects.
Unfortunately it is always skipping php-style-check and build-production-assets even if a change is made.

@szepeviktor
Copy link
Author

szepeviktor commented Feb 8, 2024

Thank you for testing.
Could you add this before the if and see its output?

git diff --diff-filter=ACMRT origin/${{ github.event.repository.default_branch }} HEAD -- web/app/themes/${{ inputs.THEME_NAME }}

@codepuncher
Copy link
Member

There is no output.
Assume this is because we are only fetching main using ref: ${{ github.event.repository.default_branch }}

@szepeviktor
Copy link
Author

szepeviktor commented Feb 8, 2024

I see! The original code only fetches github.event.repository.default_branch but does not check it out.

@szepeviktor szepeviktor closed this Feb 8, 2024
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