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

Use faster json schema validator and remove color format. #1717

Merged
merged 1 commit into from
Nov 24, 2016
Merged

Conversation

domoritz
Copy link
Member

No description provided.

@kanitw
Copy link
Member

kanitw commented Nov 24, 2016

Why remove color format?

@domoritz
Copy link
Member Author

We didn't set it consistently and so I'd rather remove it. Is there a specific use case for it?

@kanitw
Copy link
Member

kanitw commented Nov 24, 2016

I don't know. I think you added it?

@kanitw kanitw merged commit 104eb75 into master Nov 24, 2016
@kanitw kanitw deleted the dom/ajv branch November 24, 2016 20:15
@domoritz
Copy link
Member Author

I added it when we still used it to generate the properties editor in polestar.

@kanitw
Copy link
Member

kanitw commented Nov 24, 2016

@domoritz oops. Then it's not good to remove it then!

@kanitw
Copy link
Member

kanitw commented Nov 24, 2016

It's weird that you're suggesting to remove it despite the obvious utility!

@kanitw
Copy link
Member

kanitw commented Nov 24, 2016

I know we don't have property editor right now, but I can't imagine away to know if a field is color in polestar without this!

@domoritz
Copy link
Member Author

Not reliable but you could assume that stroke, fill, and color are all colors. Anyway, I'm happy to add it again if we need it but since we didn't use it consistently, I removed it.

@kanitw
Copy link
Member

kanitw commented Nov 24, 2016

Instead of removing it, the right solution is to make it consistent ....

@domoritz
Copy link
Member Author

There are two consistent states: use it nowhere and ensure that whenever we use a color, we specify the format.

@kanitw
Copy link
Member

kanitw commented Nov 24, 2016

And the "use it nowhere" is not the ideal one...

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