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

Couple of (PSR-1) improvements #9

Merged
merged 5 commits into from
Jan 13, 2015
Merged

Couple of (PSR-1) improvements #9

merged 5 commits into from
Jan 13, 2015

Conversation

bjornpost
Copy link
Contributor

Awesome move to refactor this! I've prepared a pull request which does the following:

  • I've removed the included license in every file as there's already a global LICENSE file.
  • Improve usage of whitespace to comply with PSR-1 (maybe missed a spot or 2).
  • Add type hinting for a couple of functions (@wimwinterberg made a start with this in his fork).
  • Force usage of SSL/TLS as suggested in Remove non-SSL option #6.
  • Rename a couple of classes and functions to be StudlyCaps and not StudlyCAPS.
  • Fixed an issue which caused performGet(multi='raw') to return the constant data instead of $data.

Sidenotes:

  • I think we can dump the included autoloader function if we force composer (which is the entire purpose of the project I guess).
  • If there's a roadmap/todo list I can help getting things off that list.

/cc @jorisleker

@peschee
Copy link

peschee commented Jan 13, 2015

Thanks @bjornpost I will look over this tomorrow and merge. Also, I agree with tagging initially since this will break things.

@peschee peschee closed this Jan 13, 2015
@peschee peschee reopened this Jan 13, 2015
@peschee peschee closed this Jan 13, 2015
@peschee peschee reopened this Jan 13, 2015
@peschee
Copy link

peschee commented Jan 13, 2015

Trying to trigger a travis CI build on this, but I can't seem to get it working.

peschee pushed a commit that referenced this pull request Jan 13, 2015
Seems to work fine, I ran the travis builds in a separate branch. If you care, you could add some more unit tests in another PR? Thanks for this one.
@peschee peschee merged commit 46d16cc into gridonic:master Jan 13, 2015
@peschee peschee mentioned this pull request Jan 13, 2015
@bjornpost
Copy link
Contributor Author

Did you flip the switch for this project in your Travis CI profile?

@peschee
Copy link

peschee commented Jan 13, 2015

Yes, it is building fine for the branches and PRs now. However, I only added the automated travis builds after you had submitted this PR.

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