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

Some important changes #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

peter279k
Copy link
Member

@peter279k peter279k commented May 11, 2019

Changed log

  • Integrate StyleCI service to check PSR-2 coding style automatically on incoming commits and PRs.
  • Integrate the Travis CI service, please. And it can find the bugs/errors via automatic CI build.
  • Fix wrong namespace for ImageConverter class.
  • Create helper function and let convert function split from the ImageConverter class.

The completed Travis CI build log and StyleCI log are as follows:

@peter279k peter279k requested a review from jenstornell May 11, 2019 18:41
@peter279k peter279k force-pushed the psr2_coding_style branch from ca5abf2 to aca7811 Compare May 11, 2019 18:48
@peter279k peter279k changed the title Add StyleCI to check PSR-2 coding style Some important changes May 11, 2019
@peter279k peter279k force-pushed the psr2_coding_style branch from aca7811 to 51b5de4 Compare May 11, 2019 18:53
@jenstornell
Copy link
Collaborator

  1. Now I got confused. I thought we agreed on not using jens in the namespace, but now it's back again? namespace Jens\ImageConverter; Better shorter namespace? #6
  2. About splitting the class and the helper method into two files. It's good from a developer perspective, but from a user perspective we need to include two files instead of one, in every project. Especially if Composer is not used. I will think about it, but it has some pros and cons.

@peter279k
Copy link
Member Author

peter279k commented May 12, 2019

  1. Now I got confused. I thought we agreed on not using jens in the namespace, but now it's back again? namespace Jens\ImageConverter; Better shorter namespace? #6
  2. About splitting the class and the helper method into two files. It's good from a developer perspective, but from a user perspective we need to include two files instead of one, in every project. Especially if Composer is not used. I will think about it, but it has some pros and cons.
  1. I think we can revert the namespace to the ImageConverter. And I'm sorry I almost forget this.
  2. Without composer, and we can rpovide the autoload.php for developers to load related ImageConverter automatically.

@peter279k peter279k force-pushed the psr2_coding_style branch from 51b5de4 to 9574c77 Compare May 12, 2019 09:33
@peter279k peter279k requested a review from lablnet May 22, 2019 14:51
@lablnet
Copy link
Collaborator

lablnet commented Sep 8, 2020

This looks fine to me

@spekulatius
Copy link

Hey @jenstornell

any change this could be reviewed and merged? I'd like to use the package :)

Cheers,
Peter

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.

4 participants