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

Add support for orders & invoice #8

Merged
merged 7 commits into from
Mar 21, 2019
Merged

Conversation

viezel
Copy link

@viezel viezel commented Mar 21, 2019

  • Add support for orders & invoice
  • phpunit and unit tests

@viezel
Copy link
Author

viezel commented Mar 21, 2019

@sabas ready for review

@davidvandertuijn
Copy link
Contributor

Quick suggestions:

  • Reference to unece.org documentation instead of stylusstudio.com
  • Use mixed in @param mixed $var when type is unknown ?

@viezel
Copy link
Author

viezel commented Mar 21, 2019

@davidvandertuijn sure ill change the reference.
Regarding the mixed - well if you ask me it would rather go full strict type, but maybe to big of a refactor. So where do you want the change?

@sabas
Copy link
Contributor

sabas commented Mar 21, 2019

Thank you @viezel, I'll try to merge this night..

Just to ask also to @davidvandertuijn I pulled the changes in my client code some days ago and I noticed it failed with a type error when providing arrays with ints as it was annotated with string in the parameters, so I changed the default values from null to the previous value as string (15th march commit)

Do you want access to the repository or to the org?

@viezel
Copy link
Author

viezel commented Mar 21, 2019

Sure @sabas That would be neat to gain access. Then I could setup travis to automatically run tests for this

@sabas sabas merged commit 506bb52 into php-edifact:master Mar 21, 2019
@viezel
Copy link
Author

viezel commented Mar 22, 2019

@sabas thanks for merging it in.
can you grant me access to the repo, so I can tag the release so we follow semvar.

@sabas
Copy link
Contributor

sabas commented Mar 22, 2019

@viezel done please try

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.

3 participants