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

Remove commit date from release zip #1118

Merged
merged 2 commits into from
Feb 11, 2020

Conversation

aaemnnosttv
Copy link
Collaborator

@aaemnnosttv aaemnnosttv commented Feb 11, 2020

Summary

This is a followup bug-fix to #1098

Addresses issue #1041

Relevant technical choices

  • Removes commit date as part of the generated zip filename
    This is due to the way git-repo-info works by parsing commit data from .git/objects/xx/... rather than running git (see When run in docker, all but sha1 is null rwjblue/git-repo-info#46). However, these objects may not exist (such as after a fresh clone) causing the commiterDate to be null and thus the zip creation errors out when formatting the date (Cannot read property 'split' of null).
  • Makes .git optional, so that the release zip can still be built without it

If .git is present (even after a fresh clone) the file format will be:
[email protected]

If .git is not present (i.e. zip download from GitHub) the file format will be:
google-site-kit.v1.2.3.zip

Checklist

  • My code is tested and passes existing unit tests.
  • My code has an appropriate set of unit tests which all pass.
  • My code is backward-compatible with WordPress 4.7 and PHP 5.6.
  • My code follows the WordPress coding standards.
  • My code has proper inline documentation.
  • I have signed the Contributor License Agreement (see https://cla.developers.google.com/).

@aaemnnosttv aaemnnosttv marked this pull request as ready for review February 11, 2020 10:28
@felixarntz felixarntz merged commit 90469d8 into develop Feb 11, 2020
@felixarntz felixarntz deleted the enhance/1041-release-zip-followup branch February 11, 2020 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants