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

[WIP]: Enable Flex declarations #69

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

goanpeca
Copy link
Contributor

No description provided.

@goanpeca goanpeca changed the title WIP Enable Flex declarations [WIP]: Enable Flex declarations Apr 27, 2020
@goanpeca
Copy link
Contributor Author

goanpeca commented Apr 27, 2020

Is it ok to enable all the flex props in this PR and add tests for all of them? or you prefer I split them in smaller PRs @freakboy3742 ?

@freakboy3742
Copy link
Member

It's depends a bit on complexity. Broadly speaking, having one PR per "significant new validator/parser" is probably a good bet, unless they are really closely aligned. However, if it's just a matter of adding a bunch of declarations (which is really just a list of constants and rule definitions, not new "logic" per se), it's fine to dump all those into a single PR.

@goanpeca goanpeca force-pushed the enh/enable-flex-declaration branch from a784f9f to f36f298 Compare April 27, 2020 20:48
@goanpeca
Copy link
Contributor Author

I guess this is ok since these enables several simple declarations and adds two shorthand methods. Will add tests for those.

Is it ok then to leave this as a single PR ?

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