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.

Sign up for GitHub

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

Jump to bottom

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
+149 −381

Conversation

btry
Copy link
Collaborator

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

Style fxes

@btry
style: move/add comments
7b3f34d 
Signed-off-by: Thierry Bugier <tbugier@teclib.com>
@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 <tbugier@teclib.com>
@btry
docs(header): remove synopsis of package
916c1ce 
this text is already in README.md

Signed-off-by: Thierry Bugier <tbugier@teclib.com>
@btry
docs(header): fix package name
f0a501d 
Signed-off-by: Thierry Bugier <tbugier@teclib.com>
@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

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

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 join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DIOHz0r DIOHz0r DIOHz0r requested changes

@ajsb85 ajsb85 ajsb85 approved these changes

@accesslint accesslint[bot] accesslint 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.

None yet
3 participants
@btry @ajsb85 @DIOHz0r