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.

Sign up for GitHub

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

Jump to bottom

Enable no-undef eslint rule #1472

Merged
merged 3 commits into from
Dec 2, 2016
Merged

Enable no-undef eslint rule #1472

merged 3 commits into from
Dec 2, 2016

Conversation

nLight
Copy link
Contributor

@nLight nLight commented Dec 2, 2016 •
edited
Loading

This PR enables no-undef eslint rules and fixes all the bugs we had while having it off :trollface:

@d2iq-mergebot
Copy link

This repo has @mesosphere-mergebot integration. You can interact with the following commands.

@mesosphere-mergebot integrate-it
@mesosphere-mergebot ship-it
@mesosphere-mergebot test-it

@nLight nLight changed the title Enable no-undef eslin rule Enable no-undef eslint rule Dec 2, 2016
@nLight
Copy link
Contributor Author

nLight commented Dec 2, 2016

That'd be funny if it was true, so retest this please

@nLight
Copy link
Contributor Author

nLight commented Dec 2, 2016

retest this please 🙏

orlandohohmeier
orlandohohmeier suggested changes Dec 2, 2016
View reviewed changes
plugins/nodes/src/js/pages/nodes/nodes-grid/NodesGridContainer.js
@@ -11,6 +11,7 @@ import QueryParamsMixin from '../../../../../../../src/js/mixins/QueryParamsMixi

const MAX_SERVICES_TO_SHOW = 32;
const METHODS_TO_BIND=['handleShowServices'];
const OTHER_SERVICES_COLOR = 32;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wonder if 32 is the correct value...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally it was 8 and was equal to MAX_SERVICES_TO_SHOW which also was 8

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and as I understood it paints up to 32 services in different colors and everything else go with the same color. OTHER_SERVICES_COLOR is actually color index here:

screen shot 2016-12-02 at 18 26 53

Copy link
Contributor

Choose a reason for hiding this comment

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

@rcorral could you please verify

Copy link
Contributor

Choose a reason for hiding this comment

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

this is correct. All the colors are defined in src/styles/components/nodes-grid-view/styles.less

@orlandohohmeier
Copy link
Contributor

orlandohohmeier commented Dec 2, 2016 •
edited
Loading

Feel free to merge once @rcorral verified the OTHER_SERVICES_COLOR value. Thanks a lot for taking care of this one. 💯 ✨

@rcorral rcorral merged commit eeb6d46 into master Dec 2, 2016
@rcorral rcorral deleted the drozhkov/feature/no-issue/eslint-no-undef branch December 2, 2016 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rcorral rcorral rcorral left review comments

@orlandohohmeier orlandohohmeier orlandohohmeier approved these changes

Assignees

@orlandohohmeier orlandohohmeier

@wavesoft wavesoft

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@nLight @d2iq-mergebot @orlandohohmeier @rcorral @wavesoft