garoa / ChuviscoBot

Bot de Telegram para os grupos oficiais do Garoa Hacker Clube
10 stars 6 forks source link

fix#86 + pep8 some files #87

Closed edinhodiluviano closed 5 years ago

felipesanches commented 5 years ago

@edinhodiluviano, tem coisa demais num PR só.

Você consegue mandar um PR novo contendo somente o fix sem alterar nada da formatação? Eu posso até incorporar algumas mudanças de formatação, mas acho melhor fazer as coisas separadamente para eu poder ler só o que foi mudado no bugfix sem ter que ficar desviando das alterações cosméticas que são volumosas.

Manda aí um PR limpo, por favor.

felipesanches commented 5 years ago

nota: eu não quero adotar cegamente alguns dos requisitos do PEP8. Por exemplo, eu gosto de espaçamento de 2 caracteres, em vez de 4 ou 8. Isso é algo que não quero mudar. Dá pra configurar os linters no Travis pra ignorar esse aspecto mas ainda detectar os outros problemas.

felipesanches commented 5 years ago

Essa configuração do linter (usamos o pylint) é feita nessas linhas aqui da configuração do Travis: https://github.com/garoa/ChuviscoBot/blob/c39a6dc28e2da25fec84f703a310daddcfd025ea/.travis.yml#L14-L18

veja que lá eu já listei algumas coisas que não quero "consertar" (WONT_FIX) e outras que talvez faça sentido mudar mais pra frente (MAYBE_LATER).

edinhodiluviano commented 5 years ago

Entendi. Tranquilo. Eu nao conhecia o pylint, vou dar uma pesquisada aqui. Vlw : )

Tentei fazer um pr so do fix mas ja tinha esquecido q nao consigo fazer na master. Se der, pode tentar puxar de la. O primeiro commit que eu fiz, na branch fix#86, tem so o fix, sem modificar outras coisas.

felipesanches commented 5 years ago

ah... agora acho que entendi qual foi a sua dificuldade. Eu sigiro que você sempre abra um novo branch local para cada feature que você vá tentar implementar (ou bugfix que vá tentar corrigir). Assim você poderá trabalhar com várias coisas em paralelo, caso necessário.

Mantenha o seu branch master sempre limpo e sincronizado com o branch master daqui (que costumamos chamar de branch master do repositório upstream).

Cada novo pull request deve ser feito a partir de um feature-branch seu para ser mesclado no master do upstream.

edinhodiluviano commented 5 years ago

Hum... entao, se eu fizer um fork limpo, abrir uma branch so pra esse issue, commit e tentar fazer o PR dessa branch direto pra master deve funcionar? (se sim, entao faço isso aqui)

felipesanches commented 5 years ago

sim, vai. Mas suponho que o seu branch master local esteja agora sujo, contendo esses commits que você mandou aqui nesse PR.

Então você vai primeiro ter que "jogar fora" os 4 commits que fez aqui. O ideal é fazer assim:

git checkout master git checkout -b issue_86_pullrequest_87 git checkout HEAD^4 git checkout -b issue_86 git cherry-pick 0122437c5c626f23ba2d0bd28ab3205315ca1cb3

edite o código para deixar ele limpo

git add -p git commit --amend git push origin issue_86

e abra um PR com o branch issue_86 pedindo pra mesclar no master upstream.

Notas:

Espero que essas instruções estejam claras e sejam úteis. Qualquer dúvida é só perguntar.

Ah! E eu escrevi essas coisas de cabeça, então há um pequeno risco de eu ter falado alguma bobagem. Se qualquer desses comandos não funcionar, pergunta aqui que eu verifico.

Happy Hacking!

edinhodiluviano commented 5 years ago

show! Acredito que consegui fazer, com algum ajuste (unicas diferenças foram pq eu tinha colocado as modificações em dois commits, ao inves de um). Maior aula de git desde que aprendi a usar o basico : )

Entretanto, ainda n consegui fazer o PR. Continua dizendo q a master esta bloqueada. Mas, agora, deve estar somente o commit com o fix na issue_86.

Pra ja adiantar, algumas diferenças a mais que tem la (e o racional):

  1. O parsing da data esta no método parse_evento ao invés de parse_data_de_evento: Modifiquei o parsing de data mas não olhei os eventos regulares, por isso deixei separado. A ideia seria, no futuro, fazer um código capaz de fazer parsing dos dois e colocar no parse_data_de_evento.
  2. Tem um regex pra retirar os comments do html antes de passar pro parsing: Assim o parsing processa somente com o conteudo que importa
  3. Print o traceback em caso de erro: Assim, cada vez que houver um erro, será logado tb o pq do erro.