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

38

Recentemente, deparei com vários projetos de Ruby (ou maioria de Ruby) no GitHub que são de código aberto quando marcado com uma ferramenta de análise de código como Rubocop , crie um monte de ofensas .

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

[The] Ruby style guide recommends best practices so that real-world Ruby programmers can write code that can be maintained by other real-world Ruby programmers. ~ Source: Ruby Style Guide

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

As pessoas apreciam a consistência , portanto, fazer esses tipos de alterações é algo que está fazendo uma coisa boa para a comunidade Ruby em geral, certo?

[The author(s) of the Ruby Style Guide] didn't come up with all the rules out of nowhere - they are mostly based on my extensive career as a professional software engineer, feedback and suggestions from members of the Ruby community and various highly regarded Ruby programming resources, such as "Programming Ruby 1.9" and "The Ruby Programming Language". ~ Source: Ruby Style Guide

Não está seguindo as convenções de estilo de codificação da comunidade e as práticas recomendadas basicamente incentivando práticas ruins ?

    
por raf 02.02.2014 / 15:08
fonte

11 respostas

66

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 - enquanto acordo geral deve ser que linhas mais curtas são melhores para leitura, 80 pode ser muito restritivo para alguns com tamanhos de tela de hoje e IDE .

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, já que são muito chatas de serem revisadas, e há uma chance de que isso possa introduzir erros. Por exemplo, uma string poderia ser alternada para aspas simples, mesmo que contivesse uma interpolação deliberada e deveria estar usando aspas duplas. Os mantenedores preferem fazer limpezas de estilo enquanto trabalham no código real para que possam verificar as mudanças de estilo e não introduzir novos bugs.

    
por 02.02.2014 / 15:25
fonte
48

Você parece motivado em grande parte pelo respeito pela autoridade da ferramenta rubocop e pelo Ruby Style Guide, que os mantenedores não podem compartilhar. Eles já têm seu próprio estilo e estão acostumados com isso, portanto, qualquer alteração afetaria todos os que trabalham no projeto, e isso é muito trabalhoso, 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 em rubocop" eles não vão pensar "Oh, 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 baseado na qualidade de seu trabalho. Se você aparecer e fizer grandes coisas e então mostrar suas preocupações com o estilo, é mais provável que eles ouçam, embora eles ainda possam dizer não. Há um código aberto dizendo "Falar é barato, mostre-me o código".

    
por 02.02.2014 / 19:22
fonte
25

Pragmatismo sobre Dogma, sempre. Guias de estilo de codificação são uma forma especialmente insidiosa de mal que desviam a atenção das preocupações arquitetônicas para o absurdo frívolo como as 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 devem ser ignorados então? Não, é válido usar as ferramentas para ter uma ideia geral do que precisa ser analisado, mas não mais.

É de se admirar com que frequência os tipos juniores confundem opinião com fatos.

    
por 02.02.2014 / 20:20
fonte
13

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

Se o projeto não é importante o suficiente para você manter seu próprio ramo, então não valeria a pena limpá-lo em primeiro lugar.

As solicitações de pull podem ser feitas 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. Depois, ofereça-se para resolver o problema para o autor. Se o autor concordar, ele atribuirá o problema a você e você terá permissão para fazer uma solicitação.

Você tocou em um assunto que eu também concordo que é um problema desenfreado no GitHub . Formatando de lado, há um número de projetos que incorretamente usam anotações e que causam estragos com 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 de solicitação 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 correção parecem ser a única solução.

    
por 02.02.2014 / 15:54
fonte
10

Extraído do site Rubocop em si (ênfase minha):

One thing has always bothered me as a Ruby developer - Python developers have a great programming style reference (PEP-8) and we never got an official guide, documenting Ruby coding style and best practices.

Por favor, entenda:

Não há um Guia Oficial de Estilo Ruby

Não estou dizendo que os guias de estilo são ruins. Mas não apenas não há um guia oficial, mas ter um guia de estilo é uma escolha feita em um nível pessoal, de projeto, de equipe e de 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 diz - é altamente improvável para manter qualquer influência com o grupo. Portanto, se você não for um colaborador ativo e respeitado para esse projeto específico, provavelmente suas sugestões serão ignoradas ou, na melhor das hipóteses, um lembrete das considerações anteriores sobre a existência de um guia de estilo. Na pior das hipóteses, será tomado como um insulto, ou como um intruso ou bikeshedder metendo o nariz onde ele não faz. pertença.

Você não pode apenas sugerir um guia de estilo?

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

Tudo bem, desde que você esteja realmente claro sobre o que você é e o que deseja realizar. Se você acredita em um determinado Guia de Estilo e acredita que é o Guia do Único Estilo, ou pelo menos melhor do que qualquer coisa que esses pagãos sem lei estejam praticando, então tudo bem também.

Mas o que as pessoas não apreciam é ser informado de que seu comportamento não está de acordo com regras não-vinculativas, não-vinculantes e largamente arbitrárias, que são 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ê está percebendo fazendo, você receberá menos de "tratamento do tapete vermelho", e mais dos "nativos zangados com lanças e um grande pote de ebulição" tratamento.

    
por 03.02.2014 / 21:40
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. Muitas vezes não há "estilo de codificação"; o estilo é qualquer coisa que a pessoa que escreveu o código em questão tenha usado. Se um arquivo foi escrito por uma pessoa diferente de outro, o estilo também pode ser diferente. Mesmo em projetos em que há um estilo de consenso, a menos que eles verifiquem esse estilo regularmente, ele geralmente não é usado.

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

Portanto, não assuma diretamente que o estilo que você vê é o estilo que o projeto deseja. Na verdade, isso provavelmente pode ser generalizado para contribuir para o código aberto em geral: não assuma que o código que um projeto de código aberto tem é o código que ele quer .

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 incomode.

  • Esta é uma questão enorme bikeshed . Todo mundo e seu irmão tem uma opinião sobre essas coisas. Conseguir que um pedido desse tipo seja mesclado pode ser difícil por causa disso.

  • Também pode ser difícil conseguir que uma solicitação de pull seja mesclada, 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 estão abertos para isso, e você está disposto a ficar com o pedido de pull para a conclusão, então vá em frente.

    
por 03.02.2014 / 03:46
fonte
2

Eu costumava ser grande em ter um bom guia de estilo, mas considerando o estado de coisas em Ruby, "eu segui em frente".

Basicamente, eu vivo com o que estou trabalhando e, de outra forma, sigo as convenções gerais que aprendi em diversos trabalhos.

Para o Ruby, que é o meu idioma preferido, eu também (na minha cabeça) dividi o estilo em minhas preferências e melhores práticas aceitas universalmente, geralmente aceitas. Coisas que são universalmente aceitas Eu posso enviar uma alteração como parte de uma solicitação de alteração para uma solicitação de problema ou de ramificação de recurso.

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

Universalmente aceito para Ruby:

  • espaços não guias
  • 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 do end para blocos com várias linhas.
  • CONSTANTS_ARE_IN_ALL_CAPS
  • Use instruções predicativas ( cond ? true : false ) sobre if then se a expressão couber em uma linha.

Preferências pessoais:

  • Comprimento de linha de 120 linhas de caracteres
  • As instruções
  • case when são recuadas 2 da instrução case.
  • Use aspas simples, a menos que o dobro seja necessário.

Nenhum acordo:

  • Declarações de casos
  • Comprimento da linha

Práticas recomendadas:

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

Finalmente, como os outros detalharam, você deve perguntar primeiro. No final do dia, a chave para o estilo é a comunicação entre os desenvolvedores e a sensibilidade para os outros. Por exemplo, se eu quiser fazer uma mudança de estilo em um projeto de código aberto, eu geralmente faço uma solicitação de pull para um ou dois recursos reais ou correções de bugs primeiro. Uma vez que o mantenedor me conhece e vê que eu contribuí, então eu poderia sugerir mudanças de estilo. Eu só sugiro eles embora. Por exemplo "Ao fazer outro recurso no projeto x, notei que você tem 4 recuos de espaço em alguns arquivos e fiquei me perguntando se poderia alterá-los para 2?"

    
por 01.03.2014 / 00:29
fonte
1

Although they are small and easy to fix, do you think it is okay to change the coding style of an open source project by fixing the offences and making a Pull Request?

Em suma, não!

Claro, estou assumindo aqui que estas são apenas questões de estilo e não são bugs reais - pedidos de pull para o último sempre seria sensato (IMO.) Eu também estou supondo que você é um estranho para o projeto e não em contato com as pessoas que já o mantêm.

No entanto, em qualquer idioma, há sempre algumas pessoas que têm suas preferências que diferem do guia de estilo, para melhor ou para pior, e que tentam impor essas mudanças de estilo a pessoas que você não conhece. e um projeto que você não faz parte de, se nada mais pode parecer um pouco rude. Afinal de contas - o que você está conseguindo (na realidade) se o pedido foi aceito? Com toda a probabilidade, se os membros de um projeto quisessem mudar o estilo para algo diferente, já o teriam feito - e tudo o que você faria com esse pedido seria forçar um estilo neles 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 além de fazer "correções" no estilo. Eu diria que se você quiser abrir um diálogo com os mantenedores sobre como gostaria de trabalhar no projeto, mas achar um estilo não padronizado difícil, então tudo bem. Mas eu realmente não ficaria cegamente criando pedidos de pull para um monte de projetos que contêm apenas mudanças de estilo!

    
por 03.02.2014 / 02:30
fonte
1

QUALQUER alteração é susceptível de introduzir erros, mesmo alterando a formatação tipográfica do código.
Portanto, nenhuma alteração deve ser realizada no código, a menos que exista um caso de negócios válido e "mas o código não parece legal" ou "o código não segue os padrões de codificação" não é um caso de negócio válido. O risco é simplesmente muito grande. Agora, se você estiver fazendo grandes alterações em um arquivo de origem, colocar o arquivo inteiro de acordo com os padrões pode ser aceitável, mas provavelmente você fará pequenas alterações. Nesse caso, é quase universalmente preferível manter suas próprias alterações. alinha com o código existente, mesmo que o código existente não siga os padrões de codificação.
Heck, o código pode 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 ...

    
por 03.02.2014 / 15:41
fonte
1

Eu tenho um par de projetos .NET moderadamente bem sucedidos, e eu tive alguns PRs de pessoas que parecem ter passado pelo código com ReSharper e StyleCop e "consertado" um monte de coisas. Eu não aceito esses PRs, por algumas razões:

  • Embora muitas das mudanças sejam para melhor, algumas delas afetam negativamente o código de alguma forma, geralmente desempenho, e selecionar as partes boas é trabalho demais.
  • Mesmo que todas as alterações tenham sido benignas ou mesmo benéficas, todas as alterações em todos os arquivos em cada confirmação precisam ser revisadas e, novamente, leva muito tempo.

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

    
por 02.03.2014 / 02:06
fonte
-1

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

Se tiver um padrão diferente, você poderá oferecer ajuda para documentar ou formalizá-lo. Então, pode acontecer que haja algumas violações desse estilo, e você pode consertá-las.

    
por 03.02.2014 / 09:38
fonte