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

Fix spacing issues (4->2) in Middleware Topology js ctrl spec #8653

Merged

Conversation

mtho11
Copy link
Contributor

@mtho11 mtho11 commented May 12, 2016

Fix the spacing issues in #8642 by reformatting from 4 to 2 spaces.

BTW: I noticed spacing issues in other js files as well ;)
Maybe at some point we might just want to do a global reformat of the js files. If we just did spacing the history diffs should still be good.

@miq-bot
Copy link
Member

miq-bot commented May 12, 2016

Checked commit mtho11@3b497ee with ruby 2.2.3, rubocop 0.37.2, and haml-lint 0.16.1
0 files checked, 0 offenses detected
Everything looks good. 🏆

@abonas
Copy link
Member

abonas commented May 15, 2016

LGTM.
@mtho11 yes, the old code has still formatting issues on frontend and backend.
it is up to frontend maintainers to decide whether to do this major formatting pass on all the files.
@martinpovolny your thoughts on the topic?
@miq-bot assign @chessbyte

@miq-bot miq-bot assigned chessbyte and unassigned abonas May 15, 2016
@chessbyte chessbyte assigned martinpovolny and unassigned chessbyte May 17, 2016
@martinpovolny
Copy link
Member

@mtho11: we can make global reformat once we setup some lint checker for javascript to run on each PR. For now we can at least keep the formating.

@himdel : what's the status of a javascript style guide for ManageIQ and lint checker?

@martinpovolny martinpovolny merged commit 1b664fb into ManageIQ:master May 18, 2016
@martinpovolny martinpovolny added this to the Sprint 41 Ending May 30, 2016 milestone May 18, 2016
@himdel
Copy link
Contributor

himdel commented May 18, 2016

@martinpovolny Currently, the guide not so much, but as for the actual checking, I just need to comb through the rulesets.. should be relatively easy to just add the config files so that a check can be run manually. And as for miq-bot support - I have a testing environment set up and some begginings of the patch . . well, if I bumb up the priority, shouldn't take more than a few days..

@martinpovolny
Copy link
Member

martinpovolny commented May 18, 2016

@himdel : can you, please, put the current rules in a document in some informal way at least? I mean create a document such as the I18n guide and put the most obvious rules there, like the avoidance of global variables and size of indent. We need to be able to refer people somewhere. Maybe a reference to a javascript file that is well formatted to start with.

@himdel
Copy link
Contributor

himdel commented May 18, 2016

Will do.. a draft is available at #8781

@mtho11 mtho11 deleted the fix_spacing_issues_in_spec_4_to_2 branch July 18, 2016 01:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants