Manifest file strategy - Githubissues

humbug / phar-updater

A thing to make PHAR self-updates easy and secure

BSD 3-Clause "New" or "Revised" License

368 stars 27 forks source link

Manifest file strategy #15

Open pjcdawkins opened 8 years ago

pjcdawkins commented 8 years ago

As described in #13

Minor todos, not sure if you'd want them:

[ ] add setter methods to match current strategies a bit more closely
[ ] what about all the copyright/authorship stuff in the file docblock

padraic commented 8 years ago

@pjcdawkins Thanks for the PR! I'll review over the next week time allowing.

On your two questions:

I'd keep setter names as consistent as possible with existing API (where it makes sense).
Best practice is to include the license block at file head. Also, by all means include your name for author and copyright credit. I do not require copyright assignment, only a compatible license. If the code includes any pre-existing code, do include a line indicating the original source, author and copyright to ensure we abide by the terms of any prevailing license it may fall under, e.g. "Adapted from xxx ". It will also let me double check we're legal ;).

aik099 commented 8 years ago

Apparently the review stage took longer than expected.

pjcdawkins commented 8 years ago

Ah yes... I also have extended this a bit where I'm using it (in the Platform.sh CLI), to add support for timeouts, for upgrade notes, and for PHP version requirements: https://github.com/platformsh/platformsh-cli/blob/development/src/SelfUpdate/ManifestStrategy.php

I'll roll that into an update for this PR, if there is still interest.

ArnaudLigny commented 8 years ago

I'll roll that into an update for this PR, if there is still interest.

👍

padraic commented 7 years ago

Sorry for the delay folks - my review times suck recently :grin: We'll work around to this much faster this time.

padraic commented 7 years ago

Note: Will complete a refresh PR over the weekend - we don't have write access back to the originating branch so will be a basic merge, resolve review pts, and check the additional work noted in comments.

padraic commented 7 years ago

@pjcdawkins I've updated this PR in #37 and it should be compatible with your final version. The API has been altered to prefer setters, and SHA1 has been moved to an optional downgrade from a SHA-256 default, but otherwise the overall internals are unchanged. If/When #37 is merged, this PR will be closed. Thanks again for opening this PR!