Tudo bem fazer alterações no estilo de codificação em um projeto de código aberto que não segue as práticas recomendadas?

38

Recentemente, deparei-me com vários projetos de código aberto Ruby (ou a maioria era Ruby) no GitHub que, quando verificados com uma ferramenta de análise de código como Rubocop , criam muitas ofensas .

Agora, a maioria dessas ofensas inclui o uso de aspas duplas em vez de aspas simples (quando não é interpolação), não segue a regra de 2 espaços por nível, excede a regra de comprimento de linha de 80 caracteres ou usa {e }para blocos com várias linhas.

[O] guia de estilo Ruby recomenda práticas recomendadas para que programadores Ruby do mundo real possam escrever código que possa ser mantido por outros programadores Ruby do mundo real. ~ Fonte: Guia de Estilo Ruby

Embora sejam pequenos e fáceis de corrigir, é apropriado alterar o estilo de codificação de um projeto de código aberto, corrigindo as ofensas e fazendo uma solicitação por solicitação? Eu reconheço que alguns projetos, como o Rails, não aceitam alterações cosméticas e outros são grandes demais para "consertar" de uma só vez (o Rails, por exemplo, gera mais de 80.000 ofensas quando o Rubocop é executado - independentemente, eles têm seu próprio conjunto de códigos convenções a seguir ao contribuir). Afinal, o Ruby Style Guide existe por uma razão, juntamente com ferramentas como Rubocop.

As pessoas apreciam a consistência, de modo que fazer esse tipo de mudança é uma coisa boa para a comunidade Ruby em geral, certo?

[O (s) autor (es) do Ruby Style Guide] não criou todas as regras do nada - elas são baseadas principalmente em minha extensa carreira como engenheiro de software profissional, em comentários e sugestões de membros da comunidade Ruby e em vários recursos de programação Ruby altamente conceituados, como "Programming Ruby 1.9" e "The Ruby Programming Language". ~ Fonte: Guia de Estilo Ruby

Não seguir as convenções e práticas recomendadas de estilo de codificação da comunidade basicamente incentivando más práticas?

raf
fonte
3
Pergunta semelhante: Devo refatorar uma classe F do código climático? , mas com mais foco na arquitetura.
amon
5
Muitos desses problemas de estilo soam bem menores.
JL235 02/02
5
Veja: en.wiktionary.org/wiki/bikeshedding
kdgregory 02/02
4
@MathewFoscarini não, o que eles estão perguntando é: "se eu comprar uma arma de radar, posso decidir quais são as leis de direção locais?"
911 Jon Hanna

Respostas:

65

Pergunte aos mantenedores.

O estilo de codificação é uma discussão bastante subjetiva, e regras como comprimento máximo de linha de 80 caracteres são bastante subjetivas - embora o acordo geral deva ser que linhas mais curtas são melhores para ler, 80 pode ser muito restritivo para alguns dos tamanhos de tela e IDE atuais.

Outras regras também podem ser ignoradas de propósito. Por exemplo, um desenvolvedor pode considerar melhor o uso global de aspas duplas e estar disposto a aceitar o "risco" de interpolação acidental e um aumento extremamente pequeno no tempo de análise.

Muitos mantenedores também não gostam de grandes mudanças no estilo de codificação, pois são muito chatas para revisar e há uma chance de que isso introduza erros. Por exemplo, uma string pode ser alternada para aspas simples, mesmo que contenha uma interpolação deliberada e deva estar usando aspas duplas. Os mantenedores preferem fazer limpezas de estilo enquanto trabalham nesse código real para que possam verificar se as mudanças de estilo não introduzem novos bugs.

johannes
fonte
34
Muitos mantenedores também não gostam de grandes mudanças que não são estritamente necessárias porque tendem a criar conflitos de mesclagem que também não são estritamente necessários.
Jan Hudec
4
Você perderá a função de anotação (que cria a linha de código) no controle de origem; se houver um erro, é difícil culpar onde foi introduzido e rastrear se houver mais desse tipo de erro.
entre pares
4
Além disso - se as convenções específicas do projeto forem consistentes, você poderá criar uma configuração específica do projeto para a ferramenta de verificação de convenções e oferecer a configuração das convenções como solicitação pull. Para que os mantenedores possam verificar seu projeto usando suas configurações. (Não sei se isso é possível com a ferramenta que você usou.)
Peter Kofler
+1 nos conflitos de mesclagem. Acabei de aceitar um retrabalho no estilo de codificação e gostaria de não ter, pois causou conflitos de fusão com os PRs de outras pessoas. É fácil de resolver, mas eu teria em vez feito isso, peça por peça
Daenyth
É o IDE que é restritivo se não permitir a exibição de dois arquivos separados lado a lado, não a culpa do código abreviado.
unperson325680
48

Você parece estar motivado principalmente pelo respeito à autoridade da ferramenta rubocop e pelo Guia de estilo Ruby, que os mantenedores podem não compartilhar. Eles já têm seu próprio estilo e estão acostumados a isso; portanto, qualquer mudança afetaria todos os que trabalham no projeto, e isso é muito trabalho, especialmente se o projeto for grande.

Considere as motivações dos mantenedores. Eles (provavelmente) querem que novas pessoas se juntem a eles enviando correções de bugs, relatórios de bugs, código de trabalho e documentação bem escrita. Se você aparecer e disser "Você tem ofensas no rubocop", eles não vão pensar "Oh, bom, alguém novo para ajudar a compartilhar a carga", eles vão pensar "Quem é esse cara? Por que ele está nos dizendo o que fazer?".

Projetos de código aberto tendem a ser meritocracias: você obtém respeito com base na qualidade do seu trabalho. Se você aparecer e fizer grandes coisas e depois mostrar suas preocupações com o estilo, é mais provável que elas ouçam, embora ainda possam dizer não. Existe um código aberto dizendo "Falar é barato, me mostre o código".

Michael Shaw
fonte
1
Melhor resposta. Você deve respeitar os esforços daqueles que vieram antes ao enviar solicitações de recebimento. Mudar o estilo do projeto sob os pés de todos os desenvolvedores existentes, especialmente todos os que têm uma classificação mais alta do que você na meritocracia, dificulta a lembrança das novas regras que você introduz. Isso prejudicaria sua capacidade de manter o projeto da maneira que tem sido. Além disso, se você já estava ativo no desenvolvimento do projeto, provavelmente conheceria os pensamentos do mantenedor sobre as mudanças estilísticas e o melhor ponto do ciclo de vida do projeto para apresentá-las.
22430 Chris Keele
1
Exatamente. Por exemplo, o projeto Linux, apesar de ter um guia de estilo bastante rigoroso para novos trechos de código, não permitirá que você simplesmente envie uma grande solicitação de recebimento corrigindo problemas de estilo, porque atrapalha a fusão. Ele ainda solicita que você mantenha o estilo local, mesmo que isso signifique que seja contrário ao guia de estilo do projeto.
Rota das milhas
25

Pragmatismo sobre Dogma, sempre. Os guias de estilo de codificação são uma forma especialmente insidiosa de maldade, que desviam a atenção das preocupações arquitetônicas para bobagens frívolas como citações simples / duplas. Pergunte a si mesmo: isso realmente faz diferença?

Eles podem ser bons até certo ponto, mas no segundo em que você os trata com um fervor quase religioso, você foi longe demais. São diretrizes, sugestões, opiniões, NÃO fatos.

Eles deveriam apenas ser ignorados então? Não, há mérito em usar as ferramentas para ter uma idéia geral do que precisa ser analisado, mas não mais.

É de se admirar com que freqüência os tipos juniores confundem opinião por fato.

baweaver
fonte
11
Vale acrescentar que as alterações de reformatação tendem a criar conflitos de mesclagem, o que é uma boa razão para a maioria dos mantenedores odiarem a reformatação apenas por causa disso.
Jan Hudec
13

Você pode extrair essas alterações somente se houver um problema em aberto para corrigir a formatação. Caso contrário, inicie seu próprio ramo, e se o autor perceber que mais pessoas o utilizam, simplesmente porque é mais legível. Eles serão mesclados na ramificação por conta própria, mas estejam preparados para manter sua ramificação mesclando atualizações e corrigindo constantemente a formatação.

Se o projeto não for importante o suficiente para você manter sua própria filial, não vale a pena limpar em primeiro lugar.

Os pedidos pull podem ser tomados pessoalmente pelo autor. Não é um mecanismo para oferecer críticas, e reformatar todo o código pode ser considerado uma crítica.

Se você não deseja manter sua própria ramificação, mas deseja contribuir para um projeto. Abra um novo problema e descreva por que o formato atual está causando problemas e, em seguida, ofereça-se para resolver o problema pelo autor. Se o autor concordar, ele atribuirá o problema a você e agora você tem permissão para fazer uma solicitação de recebimento.

Você tocou em um assunto que também concordo é um problema desenfreado no GitHub . Formatando de lado, há vários projetos que usam anotações incorretamente e causam estragos em muitos IDEs. Posso pensar em três projetos amplamente populares que usam sinalizadores obsoletos incorretamente que propagam mensagens de aviso no meu IDE. Enviei solicitações pull para corrigi-las, mas os autores não usam o mesmo IDE, portanto, as solicitações pull são ignoradas.

Ramificação, fusão e fixação parecem ser a única solução.

Reactgular
fonte
Na sua opinião, você consideraria os benefícios para futuros colaboradores uma "desculpa" válida para ofensas de fixação de solicitação de recebimento? Por exemplo, para que futuros colaboradores não precisem se preocupar em examinar toda a base de códigos para entender seu estilo de codificação.
raf
@RafalChmiel Quando um autor aceita uma solicitação de recebimento, a pessoa que enviou esse recebimento é adicionada ao repositório e listada publicamente como colaboradora do projeto e permanece listada como colaboradora. Ao revisar uma solicitação de recebimento, o autor manterá isso em mente ao decidir aceitá-la. Lembre-se disso ao enviar solicitações pull. Faça coisas dignas de ser um colaborador.
Reactgular
O que eu quis dizer foi que beneficiar futuros contribuintes corrigindo ofensas seria uma razão válida para mesclar um PR com essas correções? Por que o mantenedor deve mesclar esse PR? Talvez a redação da minha pergunta estivesse um pouco errada.
raf
2
@RafalChmiel tudo o que você indica como razão é apenas sua opinião. Se é um código ofensivo que se parece com um touro, escreva seus medos em uma edição. Se o autor se defender de tal puxão, limpe suas lágrimas com um lenço de papel.
Reactgular 02/02
10

Retirado do próprio site Rubocop (ênfase minha):

Uma coisa sempre me incomodou como desenvolvedor de Ruby - os desenvolvedores de Python têm uma ótima referência de estilo de programação (PEP-8) e nunca recebemos um guia oficial , documentando o estilo de codificação de Ruby e as melhores práticas.

Por favor entenda:

Não há Guia Oficial de Estilo Ruby

Não estou dizendo que os guias de estilo são ruins. Mas não apenas não existe um guia oficial, mas ter um guia de estilo é uma escolha feita em nível pessoal, de projeto, equipe, empresa. Os guias de estilo são, para usar um termo psicológico, uma "norma social em grupo".

O que isso significa para você? Bem, se você não é considerado parte do grupo, isso significa que qualquer coisa que você - ou qualquer outro site diga - provavelmente não terá nenhuma influência sobre o grupo. Portanto, se você não é um colaborador ativo e respeitado desse projeto específico, muito provavelmente suas sugestões serão ignoradas ou, na melhor das hipóteses, serão um lembrete de considerações anteriores sobre ter um guia de estilo. Na pior das hipóteses, isso será considerado um insulto, ou como um intruso ou pedreiro enfiando o nariz onde não pertence.

Você não pode apenas sugerir um Guia de Estilo?

De fato, isso parece ser o que você quer fazer: você acredita no valor dos guias de estilo, valoriza a consistência e deseja evangelizar pela dedicação às diretrizes de estilo unificadas.

Tudo bem, contanto que você esteja realmente claro, é disso que você trata e o que deseja realizar. Se você acredita em um Guia de Estilo específico e acredita que ele é o Um Guia de Estilo Verdadeiro, ou pelo menos melhor do que aquilo que aqueles pagãos sem lei estão praticando, isso também é bom.

Mas o que as pessoas não apreciam é saber que seu comportamento não está em conformidade com regras não oficiais, não vinculativas e amplamente arbitrárias, provenientes de uma fonte que não consideram uma autoridade legítima. Quando é isso que você se propõe a fazer, ou se apenas isso é o que você percebe estar fazendo, você terá menos "o tratamento do tapete vermelho" e mais "os nativos zangados com lanças e uma panela grande". tratamento.

BrianH
fonte
3

Para oferecer uma opinião alternativa aqui, em muitos casos, essa mudança seria bem-vinda. Projetos de código aberto tendem a ter muitos autores. Geralmente não há "estilo de codificação"; o estilo é exatamente o que a pessoa que escreveu o código em questão usou. Se um arquivo foi gravado por uma pessoa diferente de outra, o estilo também pode ser diferente. Mesmo em projetos em que existe um estilo de consenso, a menos que ele verifique regularmente esse estilo, ele geralmente não é usado.

Um exemplo comum disso na minha experiência é quando alguém contribui com algum código por meio de solicitação pull, que é de qualidade relativamente baixa em termos de estilo. No entanto, o código pode funcionar. Pessoas diferentes têm opiniões diferentes sobre isso. Algumas pessoas se recusam a mesclar uma solicitação pull, a menos que o estilo seja bom. Algumas pessoas não se importam, desde que o código funcione. Algumas pessoas preferem ter um bom estilo, mas não querem assustar os colaboradores com muitos comentários "conserte o espaço em branco aqui" (eu pessoalmente sempre me sinto um pouco culpado por fazer esses comentários, mesmo sabendo que eles são para melhor boa base de código, porque parece que isso pode assustar o colaborador).

Portanto, não pense que o estilo que você vê é o estilo que o projeto deseja. De fato, isso provavelmente pode ser generalizado para contribuir com o código aberto em geral: não assuma que o código de um projeto de código aberto seja o código que ele deseja .

Você deve estar ciente de algumas coisas:

  • Algumas pessoas são religiosas sobre estilo. Se ficar claro que eles não querem se mexer, não se preocupe.

  • Este é um enorme problema de ciclismo . Todo mundo e seu irmão têm uma opinião sobre essas coisas. A mesclagem de uma solicitação pull pode ser difícil por causa disso.

  • Também pode ser difícil mesclar uma solicitação de recebimento, pois haverá conflitos de mesclagem muito rapidamente; basicamente sempre que qualquer parte da base de código que você modificou muda, mesmo que seja de maneira trivial.

Eu continuaria com a abordagem "pergunte primeiro". Se eles estiverem abertos a isso e você estiver disposto a seguir a solicitação de recebimento até a conclusão, faça isso.

asmeurer
fonte
2

Eu costumava ser grande em ter um bom guia de estilo, mas, dada a situação de Ruby, "segui em frente".

Basicamente, vivo com o que estou trabalhando e, de outra forma, sigo as convenções gerais que aprendi em vários empregos.

Para Ruby, que é a minha língua preferida, também dividi (na minha cabeça) o estilo em universalmente aceito, geralmente aceito, minhas preferências e melhores práticas. Itens universalmente aceitos Eu posso enviar uma alteração como parte de uma solicitação de alteração para um problema ou solicitação de ramificação de recurso.

Exemplos de cada estilo (na minha opinião):

Universalmente aceito para Ruby:

  • espaços não tabulações
  • dois recuos de espaço
  • method_names_use_underscores
  • As constantes começam com Caps.

Geralmente aceito:

  • Não use parênteses se não for necessário
  • Use { }para blocos de uma linha e blocos do endde várias linhas.
  • CONSTANTS_ARE_IN_ALL_CAPS
  • Use declarações predicativas ( cond ? true : false) if thense a expressão couber em 1 linha.

Preferências pessoais:

  • Comprimento da linha de 120 linhas de caracteres
  • as wheninstruções de caso são recuadas 2 da instrução de caso.
  • Use aspas simples, a menos que duas sejam necessárias.

Sem acordo:

  • Declarações de Caso
  • Comprimento da linha

Melhores práticas:

  • métodos pequenos, <= 5 linhas, se possível.
  • turmas pequenas, <= 100 linhas, se possível.
  • Evite constantes
  • O código tem testes

Finalmente, como outros detalharam, você deve perguntar primeiro. No final das contas, a chave para o estilo é a comunicação entre desenvolvedores e a sensibilidade para os outros. Por exemplo, se eu quiser fazer uma alteração de estilo em um projeto de código aberto, frequentemente solicitarei um ou dois recursos reais ou correções de erros primeiro. Uma vez que o mantenedor me sabe e vê que tenho contribuído, em seguida, eu poderia sugerir mudanças de estilo. Eu só os sugiro . Por exemplo "Enquanto fazia outro recurso no projeto x, notei que você tinha quatro recuos de espaço em alguns arquivos e fiquei pensando se poderia alterá-los para 2?"

Michael Durrant
fonte
1

Embora eles sejam pequenos e fáceis de corrigir, você acha que pode alterar o estilo de codificação de um projeto de código aberto, corrigindo as ofensas e fazendo uma solicitação de solicitação?

Em suma, não!

Claro, estou assumindo aqui que essas são apenas questões de estilo e não são bugs reais - solicitações de recebimento para o último sempre seriam sensatas (IMO). Também estou assumindo que você é um estranho no projeto e não está em contato com as pessoas que já o mantêm.

No entanto, em qualquer idioma, sempre há pessoas que têm suas preferências diferentes do guia de estilo, para melhor ou para pior, e tentam impor essas mudanças de estilo a pessoas que você não conhece e a um projeto do qual você não faz parte. de se nada mais pudesse parecer um pouco rude. Afinal - o que você está alcançando (na realidade) se a solicitação foi aceita? Com toda a probabilidade, se os membros de um projeto quisessem mudar o estilo para algo diferente, eles já teriam feito isso - e tudo o que você faria com essa solicitação seria forçar um estilo para eles que não necessariamente funcionasse melhor para os membros existentes.

Isso muda um pouco se você estiver disposto a contribuir com o projeto de outras maneiras que não sejam as "correções" de estilo. Eu diria que se você deseja abrir um diálogo com os mantenedores sobre como gostaria de trabalhar no projeto, mas acha difícil um estilo fora do padrão, tudo bem. Mas eu realmente não saía cegamente criando solicitações pull para vários projetos que contêm apenas mudanças de estilo!

berry120
fonte
1

QUALQUER alteração é suscetível de introduzir erros, mesmo alterando a formatação tipográfica do código.
Portanto, nenhuma alteração deve ser executada no código, a menos que exista um caso comercial válido e "mas o código não parece bom" ou "o código não segue os padrões de codificação" não é um caso comercial válido. O risco é simplesmente muito grande. Caramba, o código poderia muito bem ser o resultado de um gerador de código e, às vezes, ser regenerado. Geradores de código são notórios por produzir código feio ...
Agora, se você estiver fazendo grandes alterações em um arquivo de origem, é aceitável colocar o arquivo inteiro em conformidade com os padrões, mas provavelmente você fará pequenas alterações. Nesse caso, é quase universalmente preferível manter suas próprias alterações em alinhar com o código existente, mesmo que o código existente não siga os padrões de codificação.

jwenting
fonte
1

Tenho alguns projetos .NET com êxito moderado e tive alguns PRs de pessoas que parecem ter passado pelo código com ReSharper e StyleCop e "consertado" um monte de coisas. Não aceito esses PRs por alguns motivos:

  • Embora muitas das mudanças sejam para melhor, algumas delas impactam negativamente o código de alguma forma, geralmente no desempenho, e escolher as partes boas é um trabalho árduo demais.
  • Mesmo que todas as alterações fossem benignas ou benéficas, todas as alterações em todos os arquivos de cada commit precisam ser revisadas e, novamente, isso leva muito tempo.

Dito isto, se alguém quisesse adicionar uma verificação de erro ou comentários de documentos melhores, eu aceitaria esse PR em um piscar de olhos.

Mark Rendle
fonte
-1

Você precisa descobrir se os mantenedores consideram essas violações do estilo de codificação um defeito ou se o projeto possui um padrão diferente para o estilo de codificação.

Se ele tiver um padrão diferente, você pode oferecer ajuda para documentá-lo ou formalizá-lo. Então, pode haver algumas violações desse estilo, e você pode corrigi-las.

Tim
fonte
isso parece não oferecer nada valioso sobre outra resposta publicada várias horas antes desta
gnat