thervh70 / ContextProject_RDD

1 stars 0 forks source link

138 get username #149

Closed MathiasMeuleman closed 8 years ago

MathiasMeuleman commented 8 years ago

Will close #138

This issue will finalize the analytics integration. The current username is save in local storage and is now being logged to the database (instead of the default 'Travis'). Also the front-end has been updated to link to your personal analytics page (instead of the default 'Travis')

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.2%) to 93.384% when pulling 0e3240686b5dd4031a211836e5f4c5efaf4bb87a on 138-get-username into e2e964ed803d2591de3adfdd47a2d1a055a2c0c1 on dev.

thervh70 commented 8 years ago

Functionality is working like it should! However I have left some comments on your code. Please fix the issues I have addressed.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.252% when pulling a778e1e27ddd94d9c42a9989bcacab02eb5a35b0 on 138-get-username into e2e964ed803d2591de3adfdd47a2d1a055a2c0c1 on dev.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.315% when pulling cdc5522546406f2f7f35df9e5206745330692023 on 138-get-username into e2e964ed803d2591de3adfdd47a2d1a055a2c0c1 on dev.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.315% when pulling cdc5522546406f2f7f35df9e5206745330692023 on 138-get-username into e2e964ed803d2591de3adfdd47a2d1a055a2c0c1 on dev.

mpsijm commented 8 years ago

Changes are looking good, thanks for resolving the feedback! :)

mdingjan commented 8 years ago

Although you haven't any extra test cases for your additional functions (in the Controllers), you did cover them for as much as possible. :sweat_smile: The functions inside the chrome related functions can't be reached, so we won't cover these. It would be nice though if you would create separate test cases for your methods, right?

Good job on using the chrome storage. :)

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.4%) to 93.202% when pulling 8b2730870d88e3bf76ecb0ed8c0de7cd2556b127 on 138-get-username into e2e964ed803d2591de3adfdd47a2d1a055a2c0c1 on dev.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.4%) to 93.193% when pulling 6a715ee4ce426af1f0ba756359d15c76a8265216 on 138-get-username into e2e964ed803d2591de3adfdd47a2d1a055a2c0c1 on dev.

MathiasMeuleman commented 8 years ago

I believe I have resolved all feeback. The thing about testibility: for tests to work I would have to simulate chages inside the real (so no mocked ones) chrome.storage. This is not really an option, will be documented. For the only thing that could be tested (init username) a test has been added.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.3%) to 93.261% when pulling b2b84bfe2df563f111918deacb045886f3a8850f on 138-get-username into e2e964ed803d2591de3adfdd47a2d1a055a2c0c1 on dev.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.2%) to 93.383% when pulling b2b84bfe2df563f111918deacb045886f3a8850f on 138-get-username into e2e964ed803d2591de3adfdd47a2d1a055a2c0c1 on dev.

mpsijm commented 8 years ago

Is looking good again :) I'll merge now, since all feedback has been properly resolved. :)