Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Fix/python-validations #1730

Closed

Conversation

michaelhly
Copy link
Contributor

@michaelhly michaelhly commented Mar 25, 2019

Description

Fix issue described in #1727.

Testing instructions

run ./setup.py test in sra_client directory.

Types of changes

Bug fix (non-breaking change which fixes an issue)

Checklist:

  • Prefix PR title with [WIP] if necessary.
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Add new entries to the relevant CHANGELOG.jsons.

@michaelhly michaelhly requested a review from feuGeneA as a code owner March 25, 2019 05:50
@michaelhly michaelhly mentioned this pull request Mar 25, 2019
4 tasks
@michaelhly michaelhly force-pushed the fix/python-validations branch 2 times, most recently from 93310c8 to 2d1f365 Compare March 25, 2019 06:41
Copy link
Contributor

@feuGeneA feuGeneA left a comment

Choose a reason for hiding this comment

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

Let's not change the json schema in this PR. For one thing you'd also have to change the copy over in monorepo/packages/json-schemas (that's why the static-tests-python CI task is failing). But more broadly I think that change warrants a broader review/discussion with the 0x team. If this PR is just updating the sra-client regexes then I'm comfortable Approving it myself, without waiting to discuss with anyone else.

"version": "1.0.1",
"changes": [
{
"note": "Fix regix validation on numeric values"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"note": "Fix regix validation on numeric values"
"note": "Fix regex validation on numeric values"

@feuGeneA
Copy link
Contributor

To clarify for anyone following along, the sra-client changes here only fix the problem described in #1727 for this client. Any other clients that get generated from the SRA spec will also face the problem described in #1727, even after this PR is merged.

@michaelhly michaelhly force-pushed the fix/python-validations branch 2 times, most recently from 64a25db to 0825184 Compare March 25, 2019 15:44
@michaelhly michaelhly force-pushed the fix/python-validations branch from 0825184 to 4f58f7f Compare March 25, 2019 15:56
@michaelhly michaelhly closed this Mar 25, 2019
@michaelhly michaelhly deleted the fix/python-validations branch April 1, 2019 01:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants