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

FIX Remove broken code to copy diff files to artifacts #28

Merged

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented May 8, 2024

Issue silverstripe/.github#235

This PR deletes the "copy dist file that was different to artifacts dir" functionality that doesn't actually work, and instead just sends exit code 2.

@GuySartorelli
Copy link
Member

GuySartorelli commented May 9, 2024

The git diff check is really there to avoid malicious actors from manually injecting payloads into dist files. Given we aren't going to accept any PRs from anyone outside the CMS Squad for CMS 4 due to the current support level, this concept seems fine.

action.yml Outdated Show resolved Hide resolved
@emteknetnz emteknetnz force-pushed the pulls/1.1/ignore-admin-js branch 2 times, most recently from 8743907 to b3d4b59 Compare May 9, 2024 02:20
@emteknetnz emteknetnz changed the title FIX Ignore differences in silverstripe/admin dist files FIX Remove broken code to copy diff files to artifacts May 9, 2024
Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@GuySartorelli GuySartorelli merged commit d59639e into silverstripe:1.1 May 9, 2024
4 checks passed
@GuySartorelli GuySartorelli deleted the pulls/1.1/ignore-admin-js branch May 9, 2024 04:21
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