Skip to content

Conversation

@rubentebogt
Copy link

Hi @chinmaypurav, verry cool package this iss!

Would really like to use this in my projects, therefor i made the following changes, i hope you like them:

  • Added basic dockblock documentation
  • Fixed one or two small bugs
  • Added basic infrastructure for Unit testing using PHPunit, coverage is not nearly a 100% but at least this gives us a start.
  • Also i added some basic functions using a trait to export the generated json

Possible improvements i think would be:

  • Adding the repo to packagist so that everyone can install.
  • Implementing 100% testing coverage
  • Implementing PSR-7 so that you could generate from existing request objects in other packages.

"type": "library",
"require": {
"php": "^8.0",
"ext-json": "*",
Copy link
Contributor

Choose a reason for hiding this comment

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

This requirement is not further required as we are on php ^8.0

PHP 8 ext-json

@chinmaypurav
Copy link
Contributor

Hi @chinmaypurav, verry cool package this iss!

Would really like to use this in my projects, therefor i made the following changes, i hope you like them:

  • Added basic dockblock documentation
  • Fixed one or two small bugs
  • Added basic infrastructure for Unit testing using PHPunit, coverage is not nearly a 100% but at least this gives us a start.
  • Also i added some basic functions using a trait to export the generated json

Possible improvements i think would be:

  • Adding the repo to packagist so that everyone can install.
  • Implementing 100% testing coverage
  • Implementing PSR-7 so that you could generate from existing request objects in other packages.

Hey @rubentebogt . Thanks for the PR. I will be reviewing it shortly.

As for your suggestions; this package is already available on packagist.org. I have not yet created a stable release, because there are a few additional changes I am working on.
This package is a dependency package for Postman Laravel which is my core focus.

@chinmaypurav
Copy link
Contributor

So @rubentebogt , I would like you to thank for your efforts towards this package. Your idea gave me some inspiration to tweak the code. There are certain issues with the PR which needs to be resolved. but yes I have merged a few of your recommended changes

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