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

Feature/check quick hover bug #5

Merged
merged 2 commits into from Nov 25, 2016
Merged

Conversation

EugeneAlforov
Copy link
Contributor

@EugeneAlforov EugeneAlforov commented Nov 23, 2016

Rebased. Had delete branch on remote.
Build has passed ok.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling e07472dfd08020f9277bc1c0581d81edf4fa13ae on feature/check-quick-hover-bug into fca1454 on master.

galkin
galkin previously requested changes Nov 23, 2016
@@ -49,10 +49,13 @@
"chai-spies": "^0.7.1",
"coveralls": "^2.11.15",
"enzyme": "^2.6.0",
"eslint": "^3.10.2",
"eslint": "3.10.2",
"eslint-config-airbnb": "13.0.0",
"eslint-config-standard": "^6.2.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this dependency? Maybe make clean up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, removed this dependency.

Copy link
Contributor

Choose a reason for hiding this comment

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

eslint-config-standard!!!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling ccd96e5b20c160f9abfeab40097cb3987ca54a47 on feature/check-quick-hover-bug into fca1454 on master.

@galkin
Copy link
Contributor

galkin commented Nov 23, 2016

@Eugen1992 please check package.json dependecies.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling bd1a220523fd869cc1430aa06acd00fe39147702 on feature/check-quick-hover-bug into fca1454 on master.

Copy link
Contributor

@galkin galkin left a comment

Choose a reason for hiding this comment

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

Please clean up commit history and splite into two commit one for change code style, second for add new functionality

"eslint-config-standard": "^6.2.1",
"eslint-config-airbnb": "13.0.0",
"eslint-plugin-import": "2.2.0",
"eslint-plugin-jsx-a11y": "2.2.3",
"eslint-plugin-promise": "^3.4.0",
"eslint-plugin-react": "^6.4.1",
"eslint-plugin-standard": "^2.0.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we need this dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In order to run tests with airbnb config

Copy link
Contributor

Choose a reason for hiding this comment

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

I commented eslint-plugin-standard which not used by airbnb

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests fail without this dependency as well. I guess airbnb config relies on this one. I might check for more details.

mouseOutHandler(currentTarget);
}
renderInnerList() {
let result;
Copy link
Contributor

Choose a reason for hiding this comment

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

do we can use return without making this unused variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can. But default eslint settings do not allow it and I personally find it more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okey.

@EugeneAlforov
Copy link
Contributor Author

Cleaned commit history.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling dabee14de8173368bbb8b1a0063f75c2704b2663 on feature/check-quick-hover-bug into fca1454 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 434aadf85562c05487aaeb69e1d3e17370dacfa3 on feature/check-quick-hover-bug into fca1454 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling d118f06e1f1f57e732929fa099e74b3fb4145b43 on feature/check-quick-hover-bug into fca1454 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 7258e93423b35ddb6e5a542bf98960e017f29046 on feature/check-quick-hover-bug into fca1454 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 1a29686259d6996ad0c31a6b44def39bf5648b99 on feature/check-quick-hover-bug into fca1454 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling dbf075f on feature/check-quick-hover-bug into fca1454 on master.

@galkin galkin merged commit fd3fd87 into master Nov 25, 2016
@galkin galkin deleted the feature/check-quick-hover-bug branch November 25, 2016 15:33
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.

None yet

3 participants