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 xmlStandalone property to XmlWriter, includes a test and documentation #15

Merged
merged 3 commits into from
Nov 8, 2023

Conversation

jbraband
Copy link
Contributor

@jbraband jbraband commented Nov 8, 2023

Spatie's spatie/array-to-xml package exposes a parameter to set the XML standalone attribute in the declaration.

This pull request adds that to saloonphp/xml-wrangler. The implementation mirrors the existing API for XmlEncoding and XmlVersion.

Copy link
Member

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

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

Awesome, thank you for adding this! Just one tiny change please!

/**
* Set the XML standalone
*/
public function setXmlStandalone(bool $xmlStandalone): XmlWriter
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public function setXmlStandalone(bool $xmlStandalone): XmlWriter
public function setXmlStandalone(bool $xmlStandalone): static

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would you like setXmlEncoding and setXmlVersion also marked with static for the return type? They are currently XmlWriter

Copy link
Contributor Author

@jbraband jbraband Nov 8, 2023

Choose a reason for hiding this comment

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

Change made, also made the change for setXmlEncoding and setXmlVersion. It just makes sense to do :) Let me know if you think otherwise

@Sammyjo20
Copy link
Member

Don't worry if php-cs-fixer fails :)

Copy link
Member

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

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

Awesome thank you @jbraband for also updating my code

@Sammyjo20 Sammyjo20 merged commit 2fefad4 into saloonphp:v1 Nov 8, 2023
9 of 10 checks passed
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