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

php artisan lychee:sync failing with Unresolvable dependency resolving bool $enableCLIFormatting #1319

Merged
merged 1 commit into from
May 10, 2022

Conversation

mederel
Copy link
Contributor

@mederel mederel commented May 9, 2022

fixes #1318

Getting the following exception:

Unresolvable dependency resolving [Parameter #1 [ <required> bool $enableCLIFormatting ]] in class App\Actions\Import\Exec

when running:

php artisan lychee:sync myfolder

@nagmat84
Copy link
Collaborator

nagmat84 commented May 9, 2022

I am not sure, if I like the change, because IMHO there is no sensible default whether CLI formatting should be enabled or not. If at all, then the default should be false, because the method is also used internally for HTTP request.

However, I am not sure, why you get this error at all. I cannot reproduce it and there are only exactly two places places where the constructor is invoked.

  1. In app/Console/Commands/Sync.php at
    $exec = new Exec(
    new ImportMode(
    $deleteImported,
    $this->option('import_via_symlink') === '1',
    $importViaSymlink,
    $this->option('resync_metadata')
    ),
    true,
    0
    );
  2. In app/Actions/Import/FromServer.php at
    $exec = new Exec($importMode, false, $this->determineMemLimit());

In both cases, the 2nd parameter is given. Are you sure that your working directory is clean and that you don't have a modified app/Console/Commands/Sync.php?

@nagmat84
Copy link
Collaborator

nagmat84 commented May 9, 2022

Oh, sorry my fault. The true error is in this line

public function handle(Exec $exec): int

If you want, could you revert your previous commit and remove the parameter Exec $exec from the method handle? That is the actual reason why it fails.

The code

 ​        ​/** 
 ​         * Execute the console command. 
 ​         * 
 ​         * @param Exec $exec 
 ​         * 
 ​         * @return int 
 ​         * 
 ​         * @throws ExternalLycheeException 
 ​         */ 
 ​        ​public​ ​function​ ​handle​(​Exec​ ​$​exec​): ​int

should be

 ​        ​/** 
 ​         * Execute the console command. 
 ​         * 
 ​         * @return int 
 ​         * 
 ​         * @throws ExternalLycheeException 
 ​         */ 
 ​        ​public​ ​function​ ​handle​(​): ​int

@mederel
Copy link
Contributor Author

mederel commented May 9, 2022

Oh, sorry my fault. The true error is in this line

public function handle(Exec $exec): int

If you want, could you revert your previous commit and remove the parameter Exec $exec from the method handle? That is the actual reason why it fails.

The code

 ​        ​/** 
 ​         * Execute the console command. 
 ​         * 
 ​         * @param Exec $exec 
 ​         * 
 ​         * @return int 
 ​         * 
 ​         * @throws ExternalLycheeException 
 ​         */ 
 ​        ​public​ ​function​ ​handle​(​Exec​ ​$​exec​): ​int

should be

 ​        ​/** 
 ​         * Execute the console command. 
 ​         * 
 ​         * @return int 
 ​         * 
 ​         * @throws ExternalLycheeException 
 ​         */ 
 ​        ​public​ ​function​ ​handle​(​): ​int

Changes applied. Honestly thank you so much for your diligence!!!

@nagmat84
Copy link
Collaborator

No problem. Does it work now?

@mederel
Copy link
Contributor Author

mederel commented May 10, 2022

No problem. Does it work now?

I have to test tonight. Will update here.

…g bool $enableCLIFormatting

fixes LycheeOrg#1318

Getting the following exception:

```
Unresolvable dependency resolving [Parameter LycheeOrg#1 [ <required> bool $enableCLIFormatting ]] in class App\Actions\Import\Exec
```

when running:

```
php artisan lychee:sync myfolder
```
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented May 10, 2022

Codecov Report

Merging #1319 (c87d9b4) into master (7a0c791) will decrease coverage by 1.20%.
The diff coverage is n/a.

@mederel
Copy link
Contributor Author

mederel commented May 10, 2022

No problem. Does it work now?

I have to test tonight. Will update here.

I could test it works well with that fix as well :-)

@nagmat84 nagmat84 merged commit 2fdd6bc into LycheeOrg:master May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

php artisan lychee:sync failing with Unresolvable dependency resolving bool $enableCLIFormatting
2 participants