sketch-hq / nicene

Additional Credo checks
MIT License
39 stars 7 forks source link

Document Ecto schemas #7

Open emibloque opened 4 years ago

emibloque commented 4 years ago

Hey there :wave:

I saw you have #2 still open, so I tried to implement a solution. I hope it helps!

Description

In order to avoid creating a function for each Ecto association, I copy the approach of @devonestes from #5.

I don't know if there is a way to get the @typedoc directly from the @type, but the idea behind the implementation is the following:

And for detecting the associations without documentation:

Disclaimer

The test warns if one field is not documented fails when it isn't the first one to be ran within the test module. For example, this passes:

mix test --seed 829661

But this doesn't pass:

mix test --seed 125872

It throws the following error:

 1) test warns if one field is not documented (Nicene.DocumentEctoSchemaTest)
     test/document_ecto_schema_test.exs:41
     ** (MatchError) no match of right hand side value: nil
     code: |> DocumentEctoSchema.run([])
     stacktrace:
       (credo) lib/credo/check.ex:222: Credo.Check.add_line_no_options/3
       (credo) lib/credo/check.ex:196: Credo.Check.format_issue/5
       (elixir) lib/enum.ex:1336: Enum."-map/2-lists^map/1-0-"/2
       test/document_ecto_schema_test.exs:84: (test)

I tried taking a look with the debugger, but I couldn't come to a conclusion of what is going wrong with Credo.

devonestes commented 4 years ago

Thanks for taking the time to put this together! We can’t merge this quite yet because we’re still getting all the legal stuff set up, and there will probably be some sort of contributor agreement that you’ll have to agree to before we can merge this, but I just wanted to say that I’m really appreciative that you took the time to do this and that you did a really good job with it!

When we do have all that stuff in place I’ll get back to you and we can finish this up.