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

If IGNORE_OTHER_FILES is true, only copy readme.txt #32

Merged
merged 3 commits into from
Jan 17, 2022

Conversation

markjaquith
Copy link
Contributor

@markjaquith markjaquith commented Oct 27, 2021

Adds an ENV variable IGNORE_OTHER_FILES which defaults to false.

If someone overrides it with true, then instead of the big complicated copying logic, this will happen:

# Copy readme.txt to /trunk
cp "$GITHUB_WORKSPACE/$README_NAME" trunk/$README_NAME

# Use $TMP_DIR as the source of truth
TMP_DIR=$GITHUB_WORKSPACE

This means that the only thing that will be committed is readme.txt (trunk and stable tag), and the assets.

This maintains full back compat, but fixes #12

@markjaquith markjaquith marked this pull request as ready for review October 27, 2021 07:04
@jeffpaul jeffpaul requested a review from helen October 27, 2021 14:47
@markjaquith
Copy link
Contributor Author

Still think this is worthwhile.

@jeffpaul jeffpaul added this to the 2.1.0 milestone Jan 5, 2022
@jeffpaul
Copy link
Member

jeffpaul commented Jan 5, 2022

@dinhtungdu @dhanendran given your work on other GH Actions, what's your take/review on the potential update here?

dinhtungdu
dinhtungdu previously approved these changes Jan 7, 2022
Copy link
Contributor

@dinhtungdu dinhtungdu left a comment

Choose a reason for hiding this comment

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

I think it's handy to copy only asset/readme. This will cause the readme/assets and the plugin source code out of sync because readme/assets are deployed independently now. But the flag is set to false by default, if developers set the flag, it's their responsibility to keep the readme/assets and plugin source in sync.

README.md Outdated Show resolved Hide resolved
@iamdharmesh
Copy link
Member

@markjaquith Thanks a lot for the PR and sorry for the delay in review.

PR looks good to me. just one minor change is needed in the readme, apart from that all good. Thanks again :)

@jeffpaul jeffpaul added needs:documentation This requires documentation. needs:refresh This requires a refreshed PR to resolve. labels Jan 7, 2022
Copy link
Member

@iamdharmesh iamdharmesh left a comment

Choose a reason for hiding this comment

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

Thanks!

@jeffpaul jeffpaul merged commit 380f651 into 10up:develop Jan 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs:documentation This requires documentation. needs:refresh This requires a refreshed PR to resolve.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

usage with composer based projects or similar
4 participants