Skip to content
This repository has been archived by the owner on Jun 11, 2023. It is now read-only.

Add Loggable #281

Merged
merged 13 commits into from
Oct 6, 2017
Merged

Add Loggable #281

merged 13 commits into from
Oct 6, 2017

Conversation

renatomassaro
Copy link
Member

@renatomassaro renatomassaro commented Oct 3, 2017

Depends on #271

Closes #278


  • Docs

TODO:

Incidental:


This change is Reviewable

@renatomassaro renatomassaro self-assigned this Oct 3, 2017
@renatomassaro renatomassaro changed the title Add Loggable [WIP] - Add Loggable Oct 3, 2017
@renatomassaro renatomassaro force-pushed the loggable branch 3 times, most recently from 1aa08df to e5b0d4d Compare October 4, 2017 06:05
This was referenced Oct 4, 2017
@renatomassaro
Copy link
Member Author

Reviewed 23 of 24 files at r1, 4 of 4 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks broke.


lib/log/model/loggable/flow.ex, line 154 at r2 (raw file):

  database, emitting the relevant `LogCreatedEvent`
  """
  def save([]),

Move the save/1 function to the EventHandler, so it stays out of the model.


test/log/event/handler/log_test.exs, line 24 at r2 (raw file):

    @tag :pending
    test "creates log"
  end

Review this, I think it's on another scope / already tested


Comments from Reviewable

@renatomassaro renatomassaro changed the title [WIP] - Add Loggable Add Loggable Oct 6, 2017
@renatomassaro
Copy link
Member Author

Reviewed 53 of 54 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@sourcelevel-bot
Copy link

Ebert has finished reviewing this Pull Request and has found:

  • 2 possible new issues (including those that may have been commented here).
  • 9 fixed issues! 🎉

You can see more details about this review at https://ebertapp.io/github/HackerExperience/Helix/pulls/281.

@renatomassaro
Copy link
Member Author

Reviewed 1 of 1 files at r4.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@renatomassaro renatomassaro merged commit 1a674b2 into HackerExperience:master Oct 6, 2017
@renatomassaro renatomassaro deleted the loggable branch October 6, 2017 14:35
@renatomassaro renatomassaro restored the loggable branch October 12, 2017 00:21
@renatomassaro renatomassaro deleted the loggable branch October 12, 2017 00:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant