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

Initial feedback on project (organization) #4

Closed
holtkamp opened this issue Aug 2, 2017 · 4 comments
Closed

Initial feedback on project (organization) #4

holtkamp opened this issue Aug 2, 2017 · 4 comments

Comments

@holtkamp
Copy link
Contributor

holtkamp commented Aug 2, 2017

Some feedback on the current approach:

  • why support both PHP 5.6 AND 7 and not only use / support PHP 7?
    • this is a new SDK, might be good to use that in your advantage by reducing need for backward compatibility of PHP 5.6
    • way better support for scalar and complex types, reducing chance of bugs / extensive unit tests
    • support for visibility (public, protected, private) of constants, which seem to be used extensively in the code
  • version number in composer.json should be omitted
  • composer.lock should probably not be committed to repository and part of .gitignore
  • .idea folder should probably not be committed to repository and part of .gitignore
  • the lib folder should "probably" be renamed to src
    • I believe this is not a "hard" rule, more of a PSR4 coding convention... lib suggests 3rd party code, while src is "your" code...
@dnl-blkv
Copy link
Contributor

dnl-blkv commented Aug 2, 2017

@holtkamp

Thanks for the feedback, we'll go through it!

I can immediately respond regarding the first point: we will eventually switch to PHP 7.0 only. We use it ourselves. However, the most recent Developer Ecosystem Survey suggests that 5.6 is still widely used (more than 7.0). Moreover, the end-of-life for it is scheduled on December 31st, 2018, which is still a way to go.

Sources

@LauLaman
Copy link
Contributor

LauLaman commented Aug 3, 2017

other suggestions:

  • Use travis to run automatic testing so each PR is tested before its merged to master.
  • Use scrutinizer to keep the code quality high
  • install the awesome badges in README.md:
    screen shot 2017-08-03 at 18 48 40

@dnl-blkv
Copy link
Contributor

dnl-blkv commented Aug 7, 2017

@LauLaman Thanks, these are valid improvements. We will implement those in a near future.

@holtkamp
Copy link
Contributor Author

holtkamp commented Sep 7, 2017

FYI: https://gophp71.org, would aim for PHP 7.1 😄

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

No branches or pull requests

5 participants