Feature/dashboard

[kagux]

still a work in progress
todo:

• hook up dashboard to a channel for live updates
• add request page with info
• hide or mark requests in progress
• cleanup history collapse related code
• update icons on toolbar to use font-awesome goodness

---

This change is 

[juanperi]

awesome. Some comments in the genserver... but otherwise nice first steps :)

---

Reviewed 79 of 79 files at r1.
Review status: all files reviewed at latest revision, 6 unresolved discussions, some commit checks failed.

---

.gitignore, line 27 at r1 (raw file):

/tmp
/doc
.DS_Store

shouldn't this things be ignored in your global gitignore instead on every repo?

---

lib/ex_debug_toolbar/application.ex, line 67 at r1 (raw file):

  defp phoenix_server? do
    Application.get_env(:phoenix, :serve_endpoints, false)
    true

leftover?

---

lib/ex_debug_toolbar/database/request_repo.ex, line 1 at r1 (raw file):

defmodule ExDebugToolbar.Database.RequestRepo do

i'm wondering if shouldn't you extract state to its own file, and then have all the accesses to the state through a "collection like" api.
like RequestsCollection with methods like get, pop, truncate(n), add.

---

lib/ex_debug_toolbar/database/request_repo.ex, line 74 at r1 (raw file):

    case do_get(id, state) do
      {:ok, request} ->
        queue = state.queue

shouldn't this removal of the queue be inside delete_request method?

---

test/lib/ex_debug_toolbar/database/janitor_test.exs, line 13 at r1 (raw file):

      Application.put_env(:ex_debug_toolbar, :max_requests, 2)
      on_exit fn ->
        Application.put_env(:ex_debug_toolbar, :max_requests, 30)

wouldn't be better to read the max_requests number before changing it, and restoring it here, instead of the harcoded number?

---

test/lib/ex_debug_toolbar/database/request_repo_test.exs, line 211 at r1 (raw file):

    end

    test "behaves correctly after deleting a request" do

what was the reason of this and the next 2 tests?

---

Comments from Reviewable

[kagux]

Review status: 69 of 81 files reviewed at latest revision, 6 unresolved discussions.

---

.gitignore, line 27 at r1 (raw file):

Previously, epilgrim (Juan Peri) wrote…

shouldn't this things be ignored in your global gitignore instead on every repo?

possibly :) any cons in having it here?

---

lib/ex_debug_toolbar/application.ex, line 67 at r1 (raw file):

Previously, epilgrim (Juan Peri) wrote…

leftover?

yeah, good catch! You have to do it to run benchmarks

---

lib/ex_debug_toolbar/database/request_repo.ex, line 1 at r1 (raw file):

Previously, epilgrim (Juan Peri) wrote…

i'm wondering if shouldn't you extract state to its own file, and then have all the accesses to the state through a "collection like" api.
like RequestsCollection with methods like get, pop, truncate(n), add.

that's an interesting option! I'll try it out

---

lib/ex_debug_toolbar/database/request_repo.ex, line 74 at r1 (raw file):

Previously, epilgrim (Juan Peri) wrote…

shouldn't this removal of the queue be inside delete_request method?

what do you mean? I'm not sure I understand where you want to put it

---

test/lib/ex_debug_toolbar/database/janitor_test.exs, line 13 at r1 (raw file):

Previously, epilgrim (Juan Peri) wrote…

wouldn't be better to read the max_requests number before changing it, and restoring it here, instead of the harcoded number?

you're right, I'll update it

---

test/lib/ex_debug_toolbar/database/request_repo_test.exs, line 211 at r1 (raw file):

Previously, epilgrim (Juan Peri) wrote…

what was the reason of this and the next 2 tests?

since these operations imply updates to queue, it was possible to make previous tests pass, but fail on these 3.

---

Comments from Reviewable