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

Adapt the SDK to the controller refactor #152

Merged
merged 12 commits into from
Dec 13, 2016
Merged

Conversation

dbengsch
Copy link
Contributor

@dbengsch dbengsch commented Dec 9, 2016

Copy link
Contributor

@xbill82 xbill82 left a comment

Choose a reason for hiding this comment

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

Good job and good luck solving the error on Travis :)

@codecov-io
Copy link

codecov-io commented Dec 13, 2016

Current coverage is 99.93% (diff: 100%)

Merging #152 into develop will increase coverage by <.01%

@@            develop       #152   diff @@
==========================================
  Files            15         15          
  Lines          1535       1536     +1   
  Methods         251        251          
  Messages          0          0          
  Branches        403        403          
==========================================
+ Hits           1534       1535     +1   
  Misses            1          1          
  Partials          0          0          

Powered by Codecov. Last update 88d5a55...9fab840

Copy link
Contributor

@scottinet scottinet left a comment

Choose a reason for hiding this comment

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

Apart from my remarks on the README file, great job :)

@@ -4,7 +4,7 @@
Official Kuzzle Javascript SDK
======

This SDK version is compatible with Kuzzle 1.0.0-RC6 and higher
This SDK version is compatible with Kuzzle 1.0.0-RC8 and higher
Copy link
Contributor

@scottinet scottinet Dec 13, 2016

Choose a reason for hiding this comment

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

As @benoitvidis said to me (and I agree): is this really necessary?
Since we won't have any real users until v1 is out, shouldn't we remove this sentence altogether?

@@ -140,6 +140,10 @@ kuzzle
});
```

## Building manually

Clone this github repository and run ``npm run build``. A ``dist`` directory will be created, containing a plain browserified version of this SDK, and a minified version.
Copy link
Contributor

Choose a reason for hiding this comment

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

I removed this section because I've rewritten the README. There is no "browserified" version anymore, and since this step is required to use the SDK in a browser, I've already detailed the procedure in the "Installation/Javascript" section.

I think this section should go.

@dbengsch
Copy link
Contributor Author

@scottinet Done

@dbengsch dbengsch merged commit a7dfef1 into develop Dec 13, 2016
@dbengsch dbengsch deleted the newControllerStructure branch December 13, 2016 17:27
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

6 participants