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

CP-1062 eslint, karma-jspm #101

Merged
merged 15 commits into from
May 17, 2016
Merged

CP-1062 eslint, karma-jspm #101

merged 15 commits into from
May 17, 2016

Conversation

valotas
Copy link
Contributor

@valotas valotas commented Sep 3, 2015

Make use of basic eslint rules and fixed warnings

@@ -0,0 +1,24 @@
{
"rules": {
"indent": [
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you make this the default 4 space indent (more consistent with other OSS JS repos from our org)?

@valotas
Copy link
Contributor Author

valotas commented Sep 6, 2015

Done

['catch'](function(e) {
throw e;
});
['catch'](function(e) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this chunk still looks like 2 space indents

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me fix that

@rmconsole2-wf
Copy link

@valotas This pull request has merge conflicts, please resolve.

@trentgrover-wf
Copy link
Collaborator

+1
@maxwellpeterson-wf @jayudey-wf

@jayudey-wf jayudey-wf changed the title eslint eslint, karma-jspm Oct 12, 2015
@jayudey-wf jayudey-wf changed the title eslint, karma-jspm CP-1062 eslint, karma-jspm Oct 12, 2015
@rmconsole2-wf
Copy link

@valotas This pull request has merge conflicts, please resolve.

@dustinlessard-wf
Copy link
Contributor

@valotas , If we intend on moving ahead with this, can you resolve the merge conflicts?

@valotas
Copy link
Contributor Author

valotas commented Apr 27, 2016

@dustinlessard-wf I almost forgot that.

I fixed the conflicts last time, but I don't feel like doing it every month or so :). So let me know if you feel like merging it and I'll fix them.

@evanweible-wf
Copy link
Contributor

@valotas would you mind merging this PR to address the merge conflicts? valotas#1

@valotas
Copy link
Contributor Author

valotas commented May 17, 2016

@evanweible-wf I've merged your pr but there were some remaining warnings that I've fixed. It looks like this PR can be merged now. Thanks a lot for the pr anyway!

@evanweible-wf
Copy link
Contributor

@dustinlessard-wf
Copy link
Contributor

+1

@maxwellpeterson-wf
Copy link
Contributor

+1

@trentgrover-wf
Copy link
Collaborator

@jayudey-wf ready for merge

@jayudey-wf
Copy link
Contributor

jayudey-wf commented May 17, 2016

QA Resource Approval: +10

  • Testing instruction (derivied from conversation with Max)
  • Dev +1's
  • Dev/QA +10 with detail of what was tested
    • passing CI
  • Unit test created/updated
  • All unit tests pass

Merging into master.

@jayudey-wf jayudey-wf merged commit 72b5fc5 into computmaxer:master May 17, 2016
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.

7 participants