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

Build: Clean working directory prior to packaging #2856

Merged
merged 2 commits into from Oct 3, 2017

Conversation

Projects
None yet
2 participants
@aduth
Member

aduth commented Oct 2, 2017

This pull request seeks to improve the plugin build packager script to ensure a clean working directory before creating the plugin archive. This seeks to avoid the case where ignored local files are accidentally included in the built package. It also adds improved messaging to the build script to indicate the current status of the build ("Downloading remote vendor scripts...", "Creating archive...", etc).

Open questions:

  • The script uses git clean -xdf to forcefully remove ignored files from the current working directory. A previous condition in the build script will check if there are any uncommitted changes, so this should only capture ignored files (build/, node_modules/, vendor/, etc). If there are concerns around removing files, we could either make this more interactive or present the user with the option to run the command for themselves. However, given the prior condition and the context in which this script is run, I don't think there's very much risk in this.
  • These changes reveal that .map files were incidentally included in the build likely as leftovers from local development, and are not generated by the production build. We should consider ignoring .map from being included in the archive, or update the production build to create sourcemap files. Currently a warning is logged that the zip command matches no .map files.

blocks/build/*.{js,map} \
components/build/*.{js,map} \
date/build/*.{js,map} \
editor/build/*.{js,map} \
element/build/*.{js,map} \
i18n/build/*.{js,map} \
utils/build/*.{js,map} \

Testing instructions:

  1. Create a dummy file in an ignored directory, e.g. echo "foo" > editor/build/foo.js
  2. Run npm run package-plugin
  3. Note the script displays that the file added in step 1 is removed
  4. After the script has finished, check the archive and verify that editor/build/foo.js within the archive does not exist
  5. Extra credit: Verify on a test site that the plugin can be installed and used from the generated archive

@aduth aduth added the Build Tooling label Oct 2, 2017

@mkaz

This comment has been minimized.

Show comment
Hide comment
@mkaz

mkaz Oct 3, 2017

Member

I tested this change and it all looks good.

My test first was with master adding a dummy file foo.js in the build directory and confirmed that it shows up in the created gutenberg.zip. Switching to this branch and building the file does not show.

Bonus, tested installing gutenberg.zip as plugin on site and works as expected.

👍

Member

mkaz commented Oct 3, 2017

I tested this change and it all looks good.

My test first was with master adding a dummy file foo.js in the build directory and confirmed that it shows up in the created gutenberg.zip. Switching to this branch and building the file does not show.

Bonus, tested installing gutenberg.zip as plugin on site and works as expected.

👍

@mkaz

mkaz approved these changes Oct 3, 2017

Looks good 👍

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Oct 3, 2017

Member

Thanks @mkaz !

Member

aduth commented Oct 3, 2017

Thanks @mkaz !

@aduth aduth merged commit 4bc2f88 into master Oct 3, 2017

3 checks passed

codecov/project 33.78% remains the same compared to d420547
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@aduth aduth deleted the update/clean-package-plugin branch Oct 3, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment