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

PT#149703867 standardjs/eslint swap #886

Merged

Conversation

AllenBW
Copy link
Member

@AllenBW AllenBW commented Aug 25, 2017

https://www.pivotaltracker.com/story/show/149703867

What just happened? https://standardjs.com/rules.html

A ss to demonstrate error reporting is as normal, browser and server console still show offending code

screen shot 2017-08-25 at 11 44 07 am

A ss showing we still build and run correctly (notice that url), ALSO super important 😆

screen shot 2017-08-25 at 12 06 51 pm

@AllenBW AllenBW added this to the Sprint 68 Ending Sep 4, 2017 milestone Aug 25, 2017
@Fryguy
Copy link
Member

Fryguy commented Aug 25, 2017

cc @martinpovolny @dclarizio @himdel Just wanted you to be aware of the proposed "rules" changes here that may not be consistent with the OPs UI.

EDIT: I see it was partially discussed in ManageIQ/manageiq#8781, so just compleing the notification loop :)

@chriskacerguis chriskacerguis self-assigned this Aug 25, 2017
@AllenBW
Copy link
Member Author

AllenBW commented Aug 25, 2017

@Fryguy yep did! and posted the pr 👍 😆 💘

@miq-bot
Copy link
Member

miq-bot commented Aug 25, 2017

This pull request is not mergeable. Please rebase and repush.

@AllenBW AllenBW force-pushed the PT/#149703867-standardjs-eslint-swap branch from 8158a05 to e7c13af Compare August 26, 2017 00:01
@miq-bot
Copy link
Member

miq-bot commented Aug 26, 2017

Checked commits AllenBW/manageiq-ui-service@8bcacf9~...e7c13af with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks fine. 👍

@chriskacerguis
Copy link
Contributor

@AllenBW could you tweak the code coverage a bit?

@AllenBW
Copy link
Member Author

AllenBW commented Aug 28, 2017

@chriskacerguis I do not believe that should be done in this pr, If we end up wanting to roll the linting change back we would be decreasing code coverage. As no specific new functionality was added there is not a new feature to write tests for.

@chriskacerguis
Copy link
Contributor

Spoke to @AllenBW and I agree with your above comment.

@chriskacerguis chriskacerguis merged commit 925ff3a into ManageIQ:master Aug 28, 2017
@AllenBW AllenBW deleted the PT/#149703867-standardjs-eslint-swap branch August 28, 2017 15:15
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.

4 participants