Skip to content
This repository has been archived by the owner on Sep 24, 2024. It is now read-only.

WIP Update headers and style refactor #55

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

WIP Update headers and style refactor #55

wants to merge 4 commits into from

Conversation

btry
Copy link
Collaborator

@btry btry commented Apr 20, 2018 •
edited by ajsb85
Loading

Style fxes

@btry
style: move/add comments
7b3f34d 
Signed-off-by: Thierry Bugier <[email protected]>
@btry btry added the style label Apr 20, 2018
@btry btry added this to the 3.0 milestone Apr 20, 2018
@btry btry self-assigned this Apr 20, 2018
@btry btry requested a review from DIOHz0r April 20, 2018 13:40
accesslint[bot]
accesslint bot previously approved these changes Apr 20, 2018
View reviewed changes
DIOHz0r
DIOHz0r previously approved these changes Apr 20, 2018
View reviewed changes
@btry
style(example): fix code style for examples
4c49000 
Signed-off-by: Thierry Bugier <[email protected]>
@btry
docs(header): remove synopsis of package
916c1ce 
this text is already in README.md

Signed-off-by: Thierry Bugier <[email protected]>
@btry
docs(header): fix package name
f0a501d 
Signed-off-by: Thierry Bugier <[email protected]>
@btry btry changed the title Move/add comments Update headers and style refactor Apr 20, 2018
DIOHz0r
DIOHz0r suggested changes Apr 20, 2018
View reviewed changes
Copy link

@DIOHz0r DIOHz0r left a comment

Choose a reason for hiding this comment

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

A lot of files have the error_reporting forced, I think is better to centralize that and have it as an configuration argument or ENV value

examples/example-create-01.php
@@ -48,8 +41,8 @@
namespace CFPropertyList;

// just in case...
error_reporting( E_ALL );
ini_set( 'display_errors', 'on' );
error_reporting(E_ALL);
Copy link

@DIOHz0r DIOHz0r Apr 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Is it really good to have this?....

Copy link
Collaborator Author

@btry btry Apr 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

These are examples to show how to use the library. They must be as simple as possible, straightforward. For me this is fine.

@ajsb85 ajsb85 changed the title Update headers and style refactor WIP Update headers and style refactor Apr 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@DIOHz0r DIOHz0r DIOHz0r requested changes

@ajsb85 ajsb85 ajsb85 approved these changes

@accesslint accesslint[bot] accesslint[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@btry btry

Labels
documentation hacktoberfest style
Projects
None yet
Milestone
3.0
Development

Successfully merging this pull request may close these issues.
3 participants
@btry @ajsb85 @DIOHz0r