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

Update tax rates documentation for fix 29472 #199

Merged
merged 5 commits into from
May 11, 2021

Conversation

Konamiman
Copy link
Contributor

woocommerce/woocommerce#27751 and woocommerce/woocommerce#29495 introduce two new fields for the tax rates: postcodes and cities. This pull request adds them to the documentation and also changes it so that:

Copy link
Contributor

@claudiosanches claudiosanches left a comment

Choose a reason for hiding this comment

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

Changes looks good, there's only one issue with the type of the field, it's an array of strings now, so I'm suggesting to change to string[] to indicate it.
Also I'm suggesting to indicate what version we deprecated or included parameters in REST API to avoid confusing in our REST API consumers.

Konamiman and others added 2 commits March 31, 2021 10:41
In the REST API entry point to update tax rates.

Co-authored-by: Claudio Sanches <[email protected]>
In the REST API entry point to update tax rates.
@Konamiman
Copy link
Contributor Author

Hi @claudiosanches, I've committed your suggestions and added the introduction+deprecation notices.

Copy link
Contributor

@claudiosanches claudiosanches left a comment

Choose a reason for hiding this comment

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

Looks great now, but I'll hold a little to merge it since we'll only implement this feature in 5.3.

@claudiosanches claudiosanches merged commit c86d230 into trunk May 11, 2021
@claudiosanches claudiosanches deleted the update/for-fix-29472 branch May 11, 2021 16:06
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