Skip to content
This repository has been archived by the owner on Mar 15, 2020. It is now read-only.

Manifest file strategy (updated) #37

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

padraic
Copy link
Collaborator

@padraic padraic commented May 13, 2017

A revision of the original #15, to work around not having write access on older PRs.

  • Update tests for PHPUnit 6
  • Address review points raised by @theofidry
  • Check setter methods to other strategies for consistency (use setters vs constructor params)
  • Implement additional features where warranted as noted in Manifest file strategy #15 (comment)
  • Update file hash checks to optionally use SHA-256 over SHA-1 (so we don't have to update them later!)
  • Where relevant, migrate to use Composer\Semver
  • Review test coverage and add where needed.
  • Update README

throw new RuntimeException(sprintf('Failed to write file: %s', $tmpFilename));
}

$tmpSha = sha1_file($tmpFilename);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure, that sha1 should be used for version verification? Considering recent findings on http://shattered.io/ it is possible to create another phar with same sha1 hash.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's on the todo list. I'm thinking about scrapping sha1 altogether since this is a new feature.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Slight backtrack. I'm allowing SHA-1, but not by default, to allow for any transitioning, e.g. platform.sh which this is based of off.

@padraic
Copy link
Collaborator Author

padraic commented May 14, 2017

@humbug/core Ready for review.

@padraic padraic changed the title [WIP] Manifest file strategy (updated) Manifest file strategy (updated) May 14, 2017
@padraic padraic added this to the 1.1.0 milestone May 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants