Skip to content

Renaming createable and updateable to 'creatable' and 'updatable'#245

Merged
lgebhardt merged 7 commits intoJSONAPI-Resources:masterfrom
adomokos:renaming-createable-and-updateable
Jun 10, 2015
Merged

Renaming createable and updateable to 'creatable' and 'updatable'#245
lgebhardt merged 7 commits intoJSONAPI-Resources:masterfrom
adomokos:renaming-createable-and-updateable

Conversation

@adomokos
Copy link
Copy Markdown
Contributor

@adomokos adomokos commented Jun 9, 2015

This was a bit tough as the client can implement the creatable_fields and updatable_fields. I tinkered with many options, using method_missing was the least invasive option.

@lgebhardt
Copy link
Copy Markdown
Contributor

Thanks for doing this. I try to keep the code coverage at 100% (minus a few select lines). You can run the coverage reports like this:

COVERAGE=true rake test

That will generate a coverage report in a coverage directory. When I run it I see 9 lines that are not being executed by the current tests, including the new method_missing method.

@adomokos
Copy link
Copy Markdown
Contributor Author

adomokos commented Jun 9, 2015

I am happy to make these a 100%, but how would you deal with the warn messages? Do you care if there are warnings in the test output or would you suggest using a test double for ActiveSupport::Deprecation?

@lgebhardt
Copy link
Copy Markdown
Contributor

Thanks. I'm happy to accept warnings in the output. In fact there are quite a few in there currently. Maybe there's a better way, but I'm not too anxious to pursue it at this point.

@adomokos
Copy link
Copy Markdown
Contributor Author

adomokos commented Jun 9, 2015

@lgebhardt: I added a couple of more tests to increase the coverage. I was able to suspend the deprecation warning. I could try to add more tesst for increasing the test coverage, but after some period of time this code should go away, and coverage should be back at 100%.

Is there a way I could exclude methods from the coverage report?

This is what I have:

coverage

@lgebhardt
Copy link
Copy Markdown
Contributor

You can wrap lines that you don't want to count towards the coverage with # :nocov:. The three lines you have remaining should be fine if excluded from the coverage count.

@adomokos
Copy link
Copy Markdown
Contributor Author

I ignored the deprecation related methods from the coverage report. This looks much better now:

100-percent

lgebhardt added a commit that referenced this pull request Jun 10, 2015
Renaming createable and updateable to 'creatable' and 'updatable'
@lgebhardt lgebhardt merged commit 134225a into JSONAPI-Resources:master Jun 10, 2015
@lgebhardt
Copy link
Copy Markdown
Contributor

Thanks!

@adomokos
Copy link
Copy Markdown
Contributor Author

Thank you, @lgebhardt! 👍

@adomokos adomokos deleted the renaming-createable-and-updateable branch June 10, 2015 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants