shinglyu / moztrap-new-ui

A new frontend UI for MozTrap that levarages on its REST API
10 stars 13 forks source link

Refactor merge case suite view #133

Closed yodalee closed 9 years ago

yodalee commented 9 years ago

This should not apply to master, but I don't know which should it goes. The PR try to merge Caseversion and Suite into a single class list. It's almost done but only the Table content left independent. I'm not quite sure is this a good design or not. Maybe the origin one is better, we just separate different kind of data into their class. Using require can solve the problem that they merge together in one file. So maybe we don't have to merge them into one Class List.

shinglyu commented 9 years ago

Thanks for the PR, you might have to wait for #129 to land first, because I believe this one will make #129 not-mergeable

yodalee commented 9 years ago

Maybe we should first make sure this PR is necessary OAO

shinglyu commented 9 years ago

I'm reading

shinglyu commented 9 years ago

Thanks for the effort, this is such a big work. I might need to get you access to the MozTrap dev site, so you can run the functional tests on your own. Let me get back to you on this.

shinglyu commented 9 years ago

@yodalee, seem like #129 breaks your PR, you might need to rebase.

yodalee commented 9 years ago

Yeah the rebase is a little crazy work.

yodalee commented 9 years ago

rebase done.

shinglyu commented 9 years ago

Sorry for the trouble, I'll review this today.

shinglyu commented 9 years ago

Test result: 3 failed, 5 error, 4 passed (yay). I'll open an account for you for testing

shinglyu commented 9 years ago

Most of the errors are due to the removal of several important routes. Fixing that will presumably fix most of them

shinglyu commented 9 years ago

LGTM, can you squash and create a new PR for the branch list-refactor? I would like to manually test this before merging to master. Thanks.

yodalee commented 9 years ago

You mean the last three commits?

shinglyu commented 9 years ago

I mean the whole thing. To this branch: https://github.com/shinglyu/moztrap-new-ui/tree/list-refactor

Or do you want to add anything?

yodalee commented 9 years ago

I see, wait a moment. I will do some squash, but only a few commits will be squash.