Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[add routes and tests] get cities by DDD and UF #35

Closed
wants to merge 9 commits into from
Closed

[add routes and tests] get cities by DDD and UF #35

wants to merge 9 commits into from

Conversation

juniorpb
Copy link
Contributor

@juniorpb juniorpb commented Feb 29, 2020

Retorna as cidades por meio do código UF ou código DDD, foi criado as seguintes rotas:
DDD: /api/cities/v1/ddd/21
UF: /api/cities/v1/uf/sp

Também foi incluso os testes e seus devidos cenários (error e success).

@vercel
Copy link

vercel bot commented Feb 29, 2020

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/brasilapi/brasilapi/4bkiu01rl
✅ Preview: https://brasilapi-git-fork-juniorpb-actions.brasilapi.vercel.app

@danrleypereira
Copy link

Por que os commits desse PR não estão na master?
Os teste passaram, né?
Não consigo fazer a requisição GET com as seguintes rotas:

https://brasilapi.com.br/api/cidades/v1/ddd/61
https://brasilapi.com.br/api/cities/v1/ddd/61
https://brasilapi.com.br/api/cities/v1/uf/sp

Acho que algum momento alguém sobrepôs as modificações. Consigo ver o histórico de commits e consigo ver a implementação da /pages/api/cities/v1/*, mas não consigo encontrar isso no repô do @filipedeschamps e nem do @juniorpb.

@paulo-santana
Copy link
Contributor

@danrleywillyan esse PR ainda não foi mergeado, e ele está apontando pra branch Actions do Filipe, então ele ainda teria que mergear da Actions pra Master internamente.

@danrleypereira
Copy link

No caso estamos esperando o @filipedeschamps dar o merge e fazer o projeto ficar atualizado?
Não deveria ser automático? Estou fazendo um checklist para contribuir com o projeto, mas pelo visto a dinâmica não está muito boa. O @juniorpb solicitou o pull request em fevereiro, até hoje nada.
Um projeto open source não deveria ser um pouco mais ágil?

@paulo-santana
Copy link
Contributor

Não acho que o repositório tenha um "dever" natural de ser ágil se não for um produto de venda. A demora pode vir por conta de diversos fatores, gerenciáveis ou não. Pelo que eu vi em alguns vídeos dele e alguns comentários aqui no repositório, ele pode estar planejando uma forma de apresentar e guiar o projeto no futuro, o que se mostrar melhor no longo prazo.

Se você precisar dessa feature com urgência, tem uma solução que não é lá muito convencional, que é fazer um fork do repositório do juniorpb, configurar ele numa conta na Vercel e utilizar essa implementação até que ela seja disponibilizada no repositório oficial.

@filipedeschamps
Copy link
Member

Muito obrigado pelo PR @juniorpb !

Sobre a demora na revisão, vocês estão certos, acho que até certo ponto tem que ter um dinamismo para manter o projeto vivo. O que eu não esperava é que ele tivesse uma tração tão rápido e sem divulgar 😂 mas agora que falei dele na série do Futuro do Desenvolvimento Web, está cada vez mais próximo de anunciar ele de forma oficial e estou voltando para revisar os PRs 👍

Estou revisando esse agora e já volto aqui 🤝

@filipedeschamps
Copy link
Member

filipedeschamps commented Jun 10, 2020

@juniorpb primeiro, sensacional a biblioteca cidades-promise eu não conhecia ela, vi que é de sua autoria, eu dei star pra apoiar!

Sobre o PR, na minha visão você trouxe duas informações muito importantes, mas que poderiam ficar em endpoints distintos. Por exemplo: me dá uma clareza saber que existe um endpoint só responsável por DDD, como por exemplo:

/api/ddd/v1/[ddd]

Responsabilidade bem clara e diferente de um outro endpoint que trata das unidades federativas do país. Este que poderia ficar em um outro endpoint, não sei muito bem como tratar, mas seria interessante talvez uma dependência entre primeiro estado, depois cidade, por exemplo:

/api/states/v1/ -> lista estados
/api/states/v1/[state] -> mostra informações sobre o estado
/api/states/v1/[state]/cities -> lista as cidades
/api/states/v1/[state]/cities/[city] -> mostra informações sobre a cidade

O que acha então de simplificarmos primeiro o PR para tratar exclusivamente do escopo DDD e depois num outro PR a gente trata dos estados e cidades?

Abração meu caro 🤝

@danielramosbh74
Copy link
Contributor

Muito obrigado pelo PR @juniorpb !

Sobre a demora na revisão, vocês estão certos, acho que até certo ponto tem que ter um dinamismo para manter o projeto vivo. O que eu não esperava é que ele tivesse uma tração tão rápido e sem divulgar mas agora que falei dele na série do Futuro do Desenvolvimento Web, está cada vez mais próximo de anunciar ele de forma oficial e estou voltando para revisar os PRs

Estou revisando esse agora e já volto aqui

Olá @filipedeschamps, fiquei sabendo de seu projeto justamente pelo vídeo que você fez no YouTube e gostaria de contribuir de alguma forma, talvez ajudando na Documentação e na organização em geral do projeto, para que ele fique cada vez mais acessível.

Vi que você já criou um "Título" "Como contribuir" dentro do arquivo README.md, mas podemos ir melhorando mais e criarmos, por exemplo, um "Contributing.md".

Li em uma outra "PR" que vocês estavam começando a padronizar as "mensagens de erros" nos arquivos JSON. Achei interessante.

Outra coisa: posso estar enganado, mas... estou achando meio estranho retornar o nome da cidade pelo DDD...

Vou dar um exemplo mais concreto: sou de Belo Horizonte, mas moro em Betim, cidade próxima que faz parte da "região metropolitana". O DDD de ambas é 31...
Pesquisando um pouquinho mais, achei meio confusa a distribuição de DDDs... vejam neste link: https://cepddd.com.br/?gclid=Cj0KCQjw0YD4BRD2ARIsAHwmKVltghYJtddK2LcKapGHl9_eoMBfF1DYXE88_TpYg0ERzj99TAPCKCgaAnQUEALw_wcB

@juniorpb
Copy link
Contributor Author

Prezados,
agradeço a todos pelo feedback, fiz os devidos ajustes conforme solicitado. Seguindo a dica do @filipedeschamps sobre deixar a responsabilidade bem clara, decidi criar uma nova PR #96 para rota de cidades por DDD. Também criei os devidos testes E2E.

Sendo assim, irei fechar essa pull request e seguir exclusivamente o escopo DDD na #96

@juniorpb juniorpb closed this Jul 11, 2020
@Caaddss Caaddss linked an issue Feb 8, 2021 that may be closed by this pull request
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.

API para Estado e Cidade
6 participants