pagarme / superbowleto

:football: A microservice to issue, register and manage boletos
MIT License
5 stars 0 forks source link

errors: Simple fix to the InternalServerError. Added basic unit tests for the custom errors #240

Closed evaporei closed 7 years ago

evaporei commented 7 years ago

Description

Only the NotFoundError was being tested, now all of the custom tests which are exported have unit tests. Also this helped to find a minor problem on the InternalServerError, which was being set with the wrong prototype.

Your checklist for this pull request

:rotating_light: Please review this items for a good pull request. :four_leaf_clover:

  1. I've read the project's Contributing Guidelines
  2. My commits are well written and follow pagarme/git-style-guide
  3. My changes are well covered by tests and logs
  4. I've updated the project docs (if needed)
  5. I fell safe about this implementation
  6. I feel comfortable with the code I wrote, and I'm not ashamed to show it to my friends

In a good pull request, everything above is true :relaxed:

GitCop commented 7 years ago

There were the following issues with your Pull Request

Check out our Git Style Guide here: https://github.com/pagarme/git-style-guide


This message was auto-generated by https://gitcop.com

wilkmaia commented 7 years ago

Show, @otaviopace! Muito obrigado pela contribuição!

Tenho um pedido, entretanto. De acordo com nosso Git Style Guide, item 2.1, Each commit should represent a single logical change.

Você inseriu a mudança lógica e os testes para ela no mesmo commit e os descreveu na sua mensagem de commit (o que acabou fazendo a mensagem ficar maior que o limite de 72 caracteres).

Isso dá margem para duas interpretações:

  1. Os testes fazem parte da lógica - neste caso, não especifique isso na mensagem de commit (termine a mensagem no InternalServerError);
  2. Os testes são uma lógica distinta - neste caso, por favor, divida o PR em dois commits :)

Novamente, muito obrigado pela ajuda! :D

evaporei commented 7 years ago

Certo Wilk! Seria melhor eu fazer outro pull request de outra branch com a divisão dos commits ou tem a sugestão de algo diferente?

wilkmaia commented 7 years ago

Você pode fazer um git rebase para ajustar o código nesta mesma branch.

Terminando, basta dar um git push --force ou git push --force-with-lease (recomendado) para sobrescrever as mudanças :)

grvcoelho commented 7 years ago

Oi @otaviopace que massa! Muito obrigado pela colaboração :)

Estamos com algum problema nos testes do Travis (algo relacionado a Docker). Mas assim que consertarmos, poderemos fazer merge deste PR.

Valeu :airplane:

evaporei commented 7 years ago

Obrigado pela ajuda @wilkmaia e @grvcoelho. Acho que fiz alguma besteira ao tentar remover meu commit e criar dois novos. Vou fechar o pull request e realizá-lo novamente. É minha primeira contribuição a um projeto open source, desculpe o transtorno.