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

Add a better Single File Application example #20424

Open
wants to merge 2 commits into
base: 7.2
Choose a base branch
from

Conversation

tacman
Copy link
Contributor

@tacman tacman commented Nov 29, 2024

The original example didn't work because it was expecting a return value. This example is a bit more real-world.

The original example didn't work because it didn't return the integer value.  I expanded the example to something that's marginally useful (and more real-world).
@tacman
Copy link
Contributor Author

tacman commented Nov 29, 2024

I also add a filename and instructions for running it. I don't understand the second part of the page, where the command can be registered. Is that for bin/console? Or if you want to have your own set of commands, like bin/my-files, where you would have file-counter and others, like bin/console has?

@alexislefebvre
Copy link
Contributor

This PR contains too many commits. You should be able to rebase it on 7.1 and remove the unwanted commits, or it may be simpler to create a new branch from 7.1 and cherry-pick the commits you want. If you can't reduce the commits for this PR, close it and open a new PR which should contain a few commits only.

@OskarStark OskarStark changed the base branch from 7.1 to 7.2 December 7, 2024 08:57
@OskarStark
Copy link
Contributor

I switched back to 7.2 target branch, we can adjust while merging, but shouldn't we target 6.4?

Comment on lines +38 to +42
Now run it with

php bin/file-counter.php

php bin/file-counter.php --all
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Now run it with
php bin/file-counter.php
php bin/file-counter.php --all
Now run it with:
.. code-block:: bash
php bin/file-counter.php
or
.. code-block:: bash
php bin/file-counter.php --all

// output arguments and options
$dir = realpath($input->getArgument('dir'));
$all = $input->getOption('all');
$finder = (new Symfony\Component\Finder\Finder())
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a use statement

Comment on lines +30 to +34
;
$count = iterator_count($finder);
$output->writeln( "$dir has $count " .
($all ? "files" : "files in source control"));
return SingleCommandApplication::SUCCESS;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
;
$count = iterator_count($finder);
$output->writeln( "$dir has $count " .
($all ? "files" : "files in source control"));
return SingleCommandApplication::SUCCESS;
;
$count = iterator_count($finder);
$output->writeln("$dir has $count " . ($all ? "files" : "files in source control"));
return SingleCommandApplication::SUCCESS;

Comment on lines +9 to +10
<?php // bin/file-counter.php
require __DIR__.'/../vendor/autoload.php';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<?php // bin/file-counter.php
require __DIR__.'/../vendor/autoload.php';
// bin/file-counter.php
<?php
require __DIR__.'/../vendor/autoload.php';

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

Successfully merging this pull request may close these issues.

4 participants