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 sort-default-props and deprecate jsx-sort-default-props #1861

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@alexzherdev
Copy link
Contributor

commented Jun 29, 2018

Pt. 1 of #1834

@alexzherdev

This comment has been minimized.

Copy link
Contributor Author

commented Jun 29, 2018

Looks like I need to fix #1817 to improve coverage.
Edit: done

@alexzherdev alexzherdev force-pushed the alexzherdev:rename-jsx-sort-default-props branch from 4dc8c4d to e212430 Jun 30, 2018

@@ -130,12 +134,6 @@ module.exports = {
checkSorted(propTypesObject.properties);
}
break;
case 'CallExpression':
const innerNode = node.arguments && node.arguments[0];

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 2, 2018

Collaborator

hm, why is this unused exactly?

Could this change be done in a separate commit/PR?

This comment has been minimized.

Copy link
@alexzherdev

alexzherdev Jul 2, 2018

Author Contributor

See discussion in #1817 (comment)
I removed it because coveralls was red and I needed to increase coverage to make it pass 🙂Otherwise I can absolutely make this change separately

This comment has been minimized.

Copy link
@ljharb

ljharb Jul 3, 2018

Collaborator

it'd be great to at least make it separate commits, even if it stays in this PR :-)

This comment has been minimized.

Copy link
@alexzherdev

alexzherdev Jul 3, 2018

Author Contributor

@ljharb looks like this change isn't necessary for coverage now, so I'll submit a separate PR when this one is in.

@alexzherdev alexzherdev force-pushed the alexzherdev:rename-jsx-sort-default-props branch from e212430 to 8d775a6 Jul 3, 2018

@ljharb

ljharb approved these changes Jul 3, 2018

@ljharb ljharb requested review from yannickcr, lencioni and EvHaus Jul 3, 2018

@ljharb ljharb added the rule label Jul 3, 2018

@alexzherdev alexzherdev force-pushed the alexzherdev:rename-jsx-sort-default-props branch from 8d775a6 to 32cb939 Jul 24, 2018

@alexzherdev

This comment has been minimized.

Copy link
Contributor Author

commented Jul 24, 2018

Just rebased to resolve conflicts

@alexzherdev alexzherdev force-pushed the alexzherdev:rename-jsx-sort-default-props branch from 32cb939 to c7fa863 Jan 12, 2019

@amannn amannn referenced this pull request Jan 30, 2019

Open

Sort React prop types #4

@VincentLanglet

This comment has been minimized.

Copy link
Contributor

commented Feb 28, 2019

If my PR is merged before yours, you'll have to add my bug fix.
#2182

@EvHaus

EvHaus approved these changes May 20, 2019

@VincentLanglet

This comment has been minimized.

Copy link
Contributor

commented May 20, 2019

@alexzherdev Since my PR #2182 was merged, you have to update the new sort-default-props, or you'll lose the fix I made.

Thanks ;)

@amannn

This comment has been minimized.

Copy link

commented Jul 5, 2019

What's the status of this? Looks like this PR was approved and the only thing left to do is to copy over the bugfix mentioned above.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Jul 5, 2019

@alexzherdev alexzherdev force-pushed the alexzherdev:rename-jsx-sort-default-props branch from c7fa863 to fa02359 Jul 6, 2019

@alexzherdev alexzherdev force-pushed the alexzherdev:rename-jsx-sort-default-props branch from fa02359 to bbb029b Jul 6, 2019

@alexzherdev

This comment has been minimized.

Copy link
Contributor Author

commented Jul 6, 2019

Done

Show resolved Hide resolved lib/rules/sort-default-props.js Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.