-
Notifications
You must be signed in to change notification settings - Fork 28
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
paraCada (foreach) #26
base: master
Are you sure you want to change the base?
Conversation
Haha, vou precisar desenterrar minhas skills de C para poder analisar esse código, já vi logo de cara que algumas coisas do código eu simplesmente não sei o que fazem, então vou pesquisar e te perguntar algumas coisas por aqui também, certo? Uma outra coisa, talvez esse código assuste um pouco algum inciante (Ou até mesmo para gente com certa experiência) então talvez seja válido colocar num local não tão a vista, e de preferência com um aviso de que o código a seguir é complexo. Talvez seja válido colocar esse pedaço de código até em um header a parte, algo do tipo "ComandosAvancados.h" Contudo a contribuição do foreach é algo importante e que sempre quisemos ter no Brasilino, conforme mapeado na #21 então iremos analisar com bastante carinho e considerar a melhor forma de adicionarmos isto, aproveito e marco o @ErickSimoes como reviwer também. |
Em teoria o header (biblioteca) já seria transparente ao usuário. Ele só precisa saber como usar as funções. |
Sim, para o uso já seria transparente sim, mas é interessante que a Brasilino também seja facilmente "entendível" e "modificável" por qualquer um, inclusive iniciante, para que os mesmos possam contribuir. |
Não vejo problema quanto a isso. Estou mais preocupado quanto a capacidade dos controladores aceitarem esse alto nivel já que não estou com meios de teste. Ao menos a compilação na ide correu sem erros pros sockets citados. |
Oi @alessonrenato! Que ótima sua contribuição! Como @OtacilioN comentou, você fez uso de recursos bem avançados (e isso é ótimo). Na época que eu tentei desenvolver essa feature senti dificuldades exatamente por isso. Vou precisar olhar com calma pra tentar entender e avaliar essa questão levantada por Otacílio. Em breve vou descrever alguns parâmetros de testes para conseguirmos ter certeza que está tudo certo na maior quantidade de plataformas possíveis. Muito obrigado! |
Eu observei que não foram implementados exemplos do uso do paraCada. Outro ponto importante é que, tanto para fins didáticos, como boa prática de implementação do projeto, os commits de correção sejam "escondidos", mesclando-os com o commit que ele pretende corrigir. Exemplo, todos os commits depois de 'foreach' são correções dele, como 'localisation: FOREACH-paraCada', 'indentacao e comentarios ' e todos os seguintes. Eles devem ser mesclados no primeiro, já que não adicionam novas funcionalidades. Quanto aos testes, tendo sido feitas as correções a cima, recomendo testarmos nessas plataformas:
Tendo sido feitos os testes, adicionamos o paraCada na documentação e ele estará pronto para o mundo. |
6d7f9dd
to
e9bd046
Compare
@OtacilioN @ErickSimoes implementei as alterações pedidas, necessita de novos testes. |
Oi @alessonrenato! Vamos realizar os testes! |
79a0735
to
aab0c80
Compare
@ErickSimoes fiz algumas correções e ajustes no arquivo de exemplo para colocar na pasta Basicos, também recoloquei um commit que tinha atropelado do repositorio original após os rebases. |
@OtacilioN @ErickSimoes atualizei o PR pra a versão atual do Upstream e corrigi um erro no arquivo de exemplo. |
@OtacilioN @ErickSimoes alguma atualização? |
@OtacilioN @ErickSimoes vcs ainda querem a funcionalidade? |
Olá @alessonrenato, foi implantada uma Action que realiza o compile para as placas suportadas por este repositório, creio que se você atualizar esta PR com a branch master será possível finalizar a implantação do comando |
Faz tanto tempo que esse PR segue pendente de revisão pelos mantenedores que nem sei se tenho habilidade pra portar esse código de baixo nível hoje mais. Não trabalho mais tanto com compilações de código e precisaria relembrar muitos conceitos. Eu lembro que ja na época escrevi todo código com arduíno CLI no konsole e VIM porque já não gostava da IDE. Usei ele pra fazer testes nas plataformas da época e fisicamente testei no UNO, NANO e MEGA que eram as que eu tinha acesso. Não estavam populares plataformas ESP ainda. |
@alessonrenato Você se importa se eu criar uma nova Pull Request, e trazendo o commit 922df92 para ela? A ideia é que, dessa forma, eu consigo criar um Merge commit da branch |
Pode fazer, sem problemas. Atualmente não to usando as ferramentas do Git. |
@alessonrenato criei uma nova Pull Request (#74), mantive o commit (922df92), para continuar a implementação com a versão mais recente do repositório. No momento que você autorizar, irei abrir a Pull Request para Revisão dos mantenedores/colaboradores. Também estarei aberto para suas opiniões, caso queira continuar com a implementação, tal como de comentários da comunidade. |
Como requisitado no issue #21 essa é uma tentativa de implementar o paraCada (foreach) na biblioteca. Testes de compilação foram positivos no UNO, MEGA e NANO.