Fix: Deploy a specific directory within repository #83
Conversation
dinhtungdu
requested review from
a team and
cadic
and removed request for
a team
dinhtungdu
added
needs:code-review
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
One change suggestion to clarify messaging but otherwise I only have a couple of questions.
| if [[ ! -e "$GITHUB_WORKSPACE/.gitattributes" ]]; then | ||
| cat > "$GITHUB_WORKSPACE/.gitattributes" <<-EOL | ||
| /$ASSETS_DIR export-ignore | ||
| /.gitattributes export-ignore | ||
| /.gitignore export-ignore | ||
| /.github export-ignore | ||
| EOL | ||
|
|
||
| # Ensure we are in the $GITHUB_WORKSPACE directory, just in case | ||
| # The .gitattributes file has to be committed to be used | ||
| # Just don't push it to the origin repo :) | ||
| git add .gitattributes && git commit -m "Add .gitattributes file" | ||
| fi |
There was a problem hiding this comment.
It might be worth ignoring .gitignore and .gitattributes in the build directory. They can be placed anywhere in a repo.
I am undecided, can you tell me why you decided not to do so?
There was a problem hiding this comment.
@peterwilsoncc I decided not to use .disignore or .gitattributes because I assume that if plugin authors use BUILD_DIR, they are using a command/script to build the plugin, so anything inside BUILD_DIR is meant to be deployed. They have control over any files copied to that folder. Using .disignore or .gitattributes here can cause confusions IMO.
| # Copy from clean copy to /trunk, excluding dotorg assets | ||
| # The --delete flag will delete anything in destination that no longer exists in source | ||
| rsync -rc "$TMP_DIR/" trunk/ --delete --delete-excluded | ||
| if [[ "$BUILD_DIR" = false ]]; then |
There was a problem hiding this comment.
What happens if BUILD_DIR = `` (empty string) or ./ (relative directory same as github repo's root directory?
What should happen?
There was a problem hiding this comment.
In 43e313a I cover more possible value of BUILD_DIR:
././some/directory
The empty string case is handled already by [[ -z "$BUILD_DIR" ]] which returns true if BUILD_DIR is unset or set to the empty string.
IMO, BUILD_DIR is set by users so ultimately, it's users' responsibility to set the correct value for it. We already cover common use-cases in this PR.
peterwilsoncc
left a comment
peterwilsoncc
left a comment
There was a problem hiding this comment.
Thanks Tung, this looks good to me.
I'm not sure how to test this so this is based on code review only, I haven't done any branch testing.
4 participants
Description of the Change
This PR supports deploying plugin files from a custom build directory by introducing a new flag
BUILD_DIR:$BUILD_DIRis set tofalse.$BUILD_DIRwill keep using that path.$BUILD_DIRwill be concatenated with$GITHUB_WORKSPACE.All files in the
BUILD_DIRwill be copied and deployed, and because those files are generated by a build command, there is no need to use.disignoreor.gitattributehere.Note: The asset files are still copied from the repository root directory.
Closes #16
Alternate Designs
#30 #56
Possible Drawbacks
Verification Process
This requires manual verification by deploying a real plugin using this branch.
Checklist:
Changelog Entry
Credits
Props @dinhtungdu @mauryaratan