Replaced the existing PHAR building implementation with Humbug/Box#1082
Replaced the existing PHAR building implementation with Humbug/Box#1082morozov merged 1 commit intophpbrew:masterfrom morozov:box
Conversation
1. Use bin/phpbrew as the PHAR stub to unify the behavior of the packaged and development versions. 2. Removed no longer relevant build scripts, files, directories and their references in .git(attributes,ignore) files. 3. Removed CONTRIBUTORS.md as outdated. Use https://github.com/phpbrew/phpbrew/graphs/contributors instead. 4. Removed HACKING.md and todo.md as outdated. 5. Removed .hhconfig as no longer effectively supported.
|
@theofidry, as a maintainer of Box, could you take a look and see if I got it right? Later, once it's done, I want to remove the built PHAR and the signature from the repository and move them to GitHub releases and rework the This way, the repository will contain only the source code. |
|
Will take a look this week. Will try tomorrow but that may be too
optimistic of me :)
…On Mon 9 Dec 2019 at 21:29, Sergei Morozov ***@***.***> wrote:
@theofidry <https://github.com/theofidry>, as a maintainer of Box, could
you take a look and see if I got it right? Later, once it's done, I want to
remove the built PHAR and the signature from the repository and move them
to GitHub releases and rework the SelfUpdate command.
This way, the repository will contain only the source code.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1082?email_source=notifications&email_token=ABHPVAPTYLY633LD7QCYLQTQX2TD7A5CNFSM4JYQ5YL2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGKSTEI#issuecomment-563423633>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABHPVANLFVLLSC6UAUKU5DLQX2TD7ANCNFSM4JYQ5YLQ>
.
|
| TEST = phpunit | ||
|
|
||
| .PHONY: build phar | ||
| $(TARGET): vendor $(shell find bin/ shell/ src/ -type f) box.json.dist .git/HEAD |
There was a problem hiding this comment.
is the shell find necessary here? Wouldn't it be enough to have bin shell src?
There was a problem hiding this comment.
I actually copy/pasted this from the Infection's makefile.
is the
shell findnecessary here? Wouldn't it be enough to havebin shell src?
In my understanding depending on the directories would not track changes in the underlying files since Make uses the mtime of the targets. Here, we don't care about the directories themselves but care about every single file inside them.
why is
.git/HEADhere?
It may be not relevant right now but would be relevant if something like Ocramius/PackageVersions was used. It would consider the current branch/tag when generating the lock file. It could be indeed omitted for now.
| "KevinGH\\Box\\Compactor\\Json", | ||
| "KevinGH\\Box\\Compactor\\Php" | ||
| ], | ||
| "check-requirements": false, |
There was a problem hiding this comment.
I guess the requirement checker is causing problems here?
There was a problem hiding this comment.
No, I just excluded it to make the result closer to what it used to be previously. When we bump the PHP requirements, I'll consider enabling it back.
There was a problem hiding this comment.
No, I just excluded it to make the result closer to what it used to be previously.
Makes total sense
When we bump the PHP requirements, I'll consider enabling it back.
IMO it's more about when you want to give it a try. Even if you don't bump the PHP requirements, it also checks extensions provided they are properly listed in the composer.json
There was a problem hiding this comment.
This is very true. It was just out of scope. I'm totally open to enabling it and making sure all required extensions are listed in composer.json.
|
Looks good to me. I have a couple of questions but it's more out of curiosity than anything suspicious/wrong |
Fixes #1067.