Skip to content

Conversation

@akirk
Copy link
Contributor

@akirk akirk commented Jul 13, 2023

Description of the Change

When you don't have a BUILD_DIR and a .distignore file, any changes inside the git directory will be ignored when committing to SVN.

This is because we use git archive HEAD which builds the archive from the git index, not the directory tree.

A workaround is to manually commit the files in the build step, see GlotPress/GlotPress#1655

Equally well, we can just commit any file changes inside the deploy script since the git changes are only local.

How to test the Change

  1. A Github repository without a .distignore file.
  2. A build script that adds or modifies files in the git repo (e.g. minfies files).
  3. A Github actions script, this is from the README:
...
    steps:
    - uses: actions/checkout@master
    - name: Build # Remove or modify this step as needed
      run: |
        npm install
        npm run build
    - name: WordPress Plugin Deploy
      uses: 10up/action-wordpress-plugin-deploy@stable
      env:
        SVN_PASSWORD: ${{ secrets.SVN_PASSWORD }}
        SVN_USERNAME: ${{ secrets.SVN_USERNAME }}

→ The built or modified files should be included in the svn release.

Changelog Entry

Fixed - Bug fix
Ensure built files are included when used without a BUILD_DIR and .distignore file.

Credits

Props @akirk

Checklist:

  • I agree to follow this project's Code of Conduct.
  • I have updated the documentation accordingly. -- No changes necessary. This just makes it conform to expected behavior.
  • I have added tests to cover my change.
  • All new and existing tests pass.

(I'll update the checklist as soon as the build checks have run)

@akirk akirk requested a review from a team as a code owner July 13, 2023 10:28
@akirk akirk requested review from iamdharmesh and removed request for a team July 13, 2023 10:28
@jeffpaul jeffpaul added this to the 2.3.0 milestone Jul 14, 2023
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 for the PR @akirk and Apologies for the delayed review. LGTM!

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

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants