search

biocore / labadmin

Administration website for the Knight Lab
4 stars 16 forks source link

Removed JavaScript, kept only Python, added tests #153

Closed qiyunzhu closed 7 years ago

qiyunzhu commented 7 years ago

The whole pm_plate_map is removed, leaving only pm_plate_list (no JavaScript). Tests for data_access.py were added. But pm_plate_list.py has no test yet. I don't know how to so far. @josenavas do you have any inspiration to share?

josenavas commented 7 years ago

@qiyunzhu coordinate with @sjanssen2 and/or look on the project on how the other handlers are being tested.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling ababb4e71961d2ee7f4d718663ce5d4bc802e3cb on qiyunzhu:plate-mapper-ui into \ on biocore:plate-mapper-kiwi**.

qiyunzhu commented 7 years ago

@josenavas Tests for the handler added.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 33b399eb131ec55a89a0b384eadbd8ff7ee69238 on qiyunzhu:plate-mapper-ui into \ on biocore:plate-mapper-kiwi**.

qiyunzhu commented 7 years ago

@ElDeveloper Many thanks for helping me setting up the system!

ElDeveloper commented 7 years ago

Yeah, no problem!

Yoshiki.

On (Oct-19-16|17:11), Qiyun Zhu wrote:

@ElDeveloper Many thanks for helping me setting up the system!

You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/biocore/labadmin/pull/153#issuecomment-254976029

qiyunzhu commented 7 years ago

Hi all! Anyone likes this PR? Tomorrow we will try to fill the QUnit with some real JavaScript. It will be great if it's already merged by then!

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling d5ee82b19a9f3e55f4f3dfaecd671c09d8265505 on qiyunzhu:plate-mapper-ui into \ on biocore:plate-mapper-kiwi**.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 0ef89513df00d845e4f84b3f1f7ff672154326c5 on qiyunzhu:plate-mapper-ui into \ on biocore:plate-mapper-kiwi**.

qiyunzhu commented 7 years ago

@josenavas Have you had a chance to look at the tests I added? Thanks! (all other parts were already code-reviewed in past PRs.)

qiyunzhu commented 7 years ago

@josenavas Thanks! I finally found myself on GitHub :)

josenavas commented 7 years ago

ping @ElDeveloper @wasade

qiyunzhu commented 7 years ago

@mortonjt thanks for reviewing! The comments are quite reasonable. I have recoded this whole test function as well as the original function. Now it tests five instances of artificially generated database records.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 0887e43982de469065b6239720c1a42ff944ae47 on qiyunzhu:plate-mapper-ui into \ on biocore:plate-mapper-kiwi**.

josenavas commented 7 years ago

@mortonjt can you check if your comments have been addressed and, if so, merge the PR?

mortonjt commented 7 years ago

I looked at the PR more carefully - the test is much more thorough. I think it is good to go.

qiyunzhu commented 7 years ago

@mortonjt Thank you so much!!

qiyunzhu commented 7 years ago

Thank you! By the way, GitHub is down again...

On Fri, Oct 21, 2016 at 10:26 AM, Jose Navas notifications@github.com wrote:

@josenavas approved this pull request.

Thanks @qiyunzhu https://github.com/qiyunzhu ! @ElDeveloper https://github.com/ElDeveloper @wasade https://github.com/wasade available for review/merge?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/biocore/labadmin/pull/153#pullrequestreview-5294370, or mute the thread https://github.com/notifications/unsubscribe-auth/AEMVN010TY6WYBGa0nEuPoh1CgFzA3Uwks5q2PXSgaJpZM4KbiVL .

mortonjt commented 7 years ago

They haven't been -- @qiyunzhu let's meet today to knock this out.

We can also raise an issue and address this later, since this is a pretty minor part

On Oct 24, 2016 8:14 AM, "Jose Navas" notifications@github.com wrote:

@mortonjt https://github.com/mortonjt can you check if your comments have been addressed and, if so, merge the PR?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/biocore/labadmin/pull/153#issuecomment-255769913, or mute the thread https://github.com/notifications/unsubscribe-auth/AD_a3Rza99RS-Q6T7E3uGY2kf1EoyoeDks5q3MtJgaJpZM4KbiVL .