Skip to content

Conversation

@rafaelqueiroz
Copy link
Contributor

@rafaelqueiroz rafaelqueiroz commented Mar 5, 2017

The goal of PR is move the project for psr-4.

  • Create a new directory structure

Move src/JasperStarter to bin/jasperstart

The library of Java it's a command line tool and not belongs to JasperPHP application It's a dependency wich can stay outside of src/

  • New directories and namespaces

Before:

/src/JasperPHP/JasperPHP.php
/src/JasperPHP/JasperPHPCommand.php
/src/jasperPHP/JasperPHPServiceProvider.php

After:

/src/JasperPHP.php
/src/Command/JasperPHPCommand.php
/src/Service/JasperPHPServiceProvider.php

  • Review the namespaces and code style of classes and update the scripts wich using jasperstart for new path.

After this I think the next step is slicing JasperPHP class in many for respect SRP. We neeed kill the God Class. But I don't know if make this here or create a new PR.

@rafaelqueiroz rafaelqueiroz changed the title Move to PSR-4 Move for PSR-4 Mar 5, 2017
Copy link
Contributor Author

@rafaelqueiroz rafaelqueiroz left a comment

Choose a reason for hiding this comment

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

Hey @geekcom I removed in JasperPHP Class $this->resource_directory attribute and $resource_directory param of constructor.

  • Have a logic if ($resource_dir) and throws Exception;
  • I searched in PHPJasper the class never uses this attribute;

The code is unecessary.
The logic for Exception can be found in execute() method, it's a duplicate code.

Copy link
Contributor Author

@rafaelqueiroz rafaelqueiroz left a comment

Choose a reason for hiding this comment

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

Hey @geekcom I refactor execute method, create a validateExecute() and addUserToCommand()

@geekcom
Copy link
Member

geekcom commented Mar 8, 2017

This PR is great @rafaelqueiroz , thanks.

@geekcom geekcom merged commit d959002 into PHPJasper:developer Mar 8, 2017
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.

2 participants