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

Implements voter registration #23

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tiagompalte
Copy link

Implementação da validação do título de eleitor 🇧🇷

@paemuri
Copy link
Owner

paemuri commented Sep 26, 2023

Boa! Agradeço profundamente a ajuda. ❤️‍🔥 Darei uma olhada mais tarde e dou um review.

Em tempos, eu não entendo muito como funciona, mas não quer colocar as coisas (labels?) do Hacktoberfest?

@tiagompalte
Copy link
Author

Parece q não tenho permissão para adicionar label ao PR

Copy link
Owner

@paemuri paemuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Primeiramente, desculpe a demora.

Deixei alguns pontos, o que acha?

Outro ponto, acho que "voter ID" (VoterID) seria mais interessante por ser menorzinho, mas não sei quão válido é. 🤔 Alguma opinião?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
voterregistration.go Outdated Show resolved Hide resolved
voterregistration_test.go Outdated Show resolved Hide resolved
voterregistration_test.go Outdated Show resolved Hide resolved
@tiagompalte
Copy link
Author

Para fonte de consulta sobre validação do titulo de eleitor segue: http://ghiorzi.org/DVnew.htm#e

Copy link
Owner

@paemuri paemuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aprovado. Como sempre, demorei muito, mas finalmente chequei as coisas aqui.
Primeiramente, agradeço pela contribuição, e por atender as alterações que eu solicitei.

Vou dar merge, mas antes queria pontuar coisas, mais como dúvidas mesmo:

  1. O site que você usou como referência também comenta como os dois últimos dígitos antes do DV devem estar entre 01 e 28. Acha que deveríamos validar isso também? Se sim, podemos fazer em outro PR sem problemas, creio.
  2. O site acima parece usar a formatação \d{8}/\d{2}, apesar de que meu doc pessoal não a tem. Talvez não deva ser oficial, mas é um ponto para pensarmos. 👀

Como mencionei, o código está ótimo, mas as discussões acima me parecem válidas. Ainda assim, farei pequenas alterações, como usar "voter ID" em todos os lugares (ao invés de "voter registration", e talvez alguns detalhes de formatação e teste.

@tiagompalte
Copy link
Author

@paemuri validação do item 1 feita

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.

3 participants