Va bene apportare modifiche allo stile di codifica su un progetto open source che non segue le best practice?

38

Recentemente, mi sono imbattuto in un certo numero di open source Ruby (o in maggioranza di Ruby) progetti su GitHub che se selezionato con uno strumento di analisi del codice come Rubocop , crea un sacco di reati .

Ora, la maggior parte di questi reati include l'utilizzo di virgolette doppie anziché virgolette singole (quando non l'interpolazione), non seguono la regola di 2 spazi per livello, superando la regola della lunghezza della linea di 80 caratteri o utilizzando { e } per blocchi a più linee.

[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

Sebbene siano piccoli e facili da correggere, è appropriato cambiare lo stile di codifica di un progetto open source correggendo i reati e facendo una richiesta di pull? Riconosco che alcuni progetti, come Rails, non accettano modifiche estetiche e alcuni sono troppo grandi per "risolvere" tutto in una volta (per esempio Rails genera oltre 80.000 infrazioni quando Rubocop viene eseguito - a prescindere, hanno il loro piccolo insieme di convenzioni di codifica da seguire quando contribuisci). Dopotutto, la Guida allo stile di Ruby esiste per un motivo insieme a strumenti come Rubocop.

Le persone apprezzano la coerenza quindi apportare questi tipi di modifiche è un po 'come fare una buona cosa per la comunità di Ruby in generale, giusto?

[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

Non seguire le convenzioni di stile di codifica della community e le best practice in pratica incoraggiando pratiche cattive ?

    
posta raf 02.02.2014 - 15:08
fonte

11 risposte

66

Chiedi ai manutentori.

Lo stile di codifica è una discussione piuttosto soggettiva e le regole come la lunghezza massima della linea di 80 caratteri sono abbastanza soggettive - mentre l'accordo generale dovrebbe essere che le righe più brevi sono più leggibili, 80 potrebbe essere troppo restrittivo per alcuni con le dimensioni dello schermo e l'IDE di oggi .

Anche altre regole possono essere ignorate di proposito. Ad esempio, uno sviluppatore potrebbe considerare meglio l'uso globale delle doppie virgolette ed essere disposto ad accettare il "rischio" di interpolazione accidentale e un aumento estremamente ridotto del tempo di analisi.

A molti manutentori non piacciono anche le grandi modifiche allo stile di codifica perché sono noiose da revisionare e c'è la possibilità che possa introdurre degli errori. Ad esempio, una stringa potrebbe essere convertita in virgolette singole, anche se conteneva un'interpolazione intenzionale e avrebbe dovuto utilizzare virgolette doppie. I manutentori preferiscono fare pulizie di stile mentre lavorano su quel codice reale in modo che possano verificare che le modifiche di stile non introducano nuovi bug.

    
risposta data 02.02.2014 - 15:25
fonte
48

Sembra che tu sia motivato in gran parte dal rispetto dell'autorità dello strumento rubocop e della guida allo stile di Ruby, che i manutentori potrebbero non condividere. Hanno già un loro stile e sono abituati a farlo, quindi qualsiasi cambiamento potrebbe interessare tutti quelli che lavorano al progetto, e questo è un sacco di lavoro, soprattutto se il progetto è grande.

Considera le motivazioni dei manutentori. Loro (probabilmente) vogliono che nuove persone si uniscano a loro inviando correzioni di bug, segnalazioni di bug, codice funzionante e documentazione ben scritta. Se ti presenti e dici "Hai offese in rubocop" non penseranno "Oh bene, qualcuno di nuovo per aiutare a condividere il carico", penseranno "Chi è questo tizio? Perché ci sta dicendo Cosa fare?".

I progetti open source tendono ad essere meritocrazie: ottieni rispetto in base alla qualità del tuo lavoro. Se ti presenti e fai grandi cose e allora fai apparire le tue preoccupazioni di stile, è più probabile che ascoltino, anche se potrebbero ancora dire di no. C'è un open source che dice "Talk is cheap, mostrami il codice".

    
risposta data 02.02.2014 - 19:22
fonte
25

Pragmatismo su Dogma, sempre. Le guide di stile di codifica sono una forma particolarmente insidiosa di malvagità che distoglie l'attenzione dalle preoccupazioni architettoniche verso frivole assurdità come le citazioni singole / doppie. Chiediti: fa davvero la differenza?

Possono essere buoni fino a un certo punto, ma nel secondo in cui li tratti con un fervore quasi religioso, sei andato troppo oltre. Sono linee guida, suggerimenti, opinioni, NON fatti.

Dovrebbero essere ignorati allora? No, c'è il merito di usare gli strumenti per avere un'idea generale di ciò che deve essere guardato, ma non di più.

È sorprendente quanto spesso i tipi junior confondano l'opinione per fatto.

    
risposta data 02.02.2014 - 20:20
fonte
13

È possibile estrarre queste modifiche solo se esiste un problema aperto per correggere la formattazione. Altrimenti inizia la tua filiale e se l'autore vede che più persone usano la tua filiale semplicemente perché è più leggibile. Si uniranno nel ramo da soli, ma sii pronto a mantenere il tuo ramo unendovi gli aggiornamenti e sistemando costantemente la formattazione.

Se il progetto non è abbastanza importante per te per mantenere il tuo ramo personale, allora non valeva la pena ripulirlo.

Le richieste di pull possono essere prese personalmente dall'autore. Non è un meccanismo per offrire critiche, e la riformattazione di tutto il codice potrebbe essere presa come critica.

Se non vuoi mantenere la tua filiale, ma vuoi contribuire a un progetto. Apri un nuovo problema e descrivi perché il formato corrente ti sta causando problemi, quindi offri di risolvere il problema per l'autore. Se l'autore è d'accordo, ti assegnerà il problema e ora hai il permesso di effettuare una richiesta di pull.

Hai toccato un argomento che sono anche d'accordo è un problema dilagante su GitHub . A parte la formattazione, ci sono una serie di progetti che utilizzano erroneamente annotazioni e che causano il caos con molti IDE. Posso pensare a tre progetti largamente popolari che usano in modo errato i flag deprecati che propagano messaggi di avviso nel mio IDE. Ho inviato richieste pull per risolverle, ma gli autori non usano lo stesso IDE, quindi le richieste pull vengono ignorate.

Branching, merging e fixing sembrano essere l'unica soluzione.

    
risposta data 02.02.2014 - 15:54
fonte
10

Tratto dal sito Rubocop (enfasi mia):

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.

Per favore capisci:

Non esiste una guida ufficiale allo stile Ruby

Non sto dicendo che le guide di stile siano cattive. Ma non solo non esiste una guida ufficiale, ma avere una guida di stile è una scelta fatta a livello personale, di progetto, di squadra, di azienda. Le guide di stile sono, per usare un termine psicologico, una "norma sociale interna al gruppo".

Che cosa significa per te? Beh, se non sei considerato parte del gruppo, significa che qualsiasi cosa tu - o qualsiasi altro sito web dica - è altamente improbabile che possa influenzare il gruppo. Quindi, se non sei un collaboratore attivo e rispettato per quel progetto specifico, molto probabilmente i tuoi suggerimenti saranno ignorati, o saranno nel migliore dei casi un promemoria delle precedenti considerazioni sull'avere una guida di stile. Nel peggiore dei casi sarà considerato un insulto o un intruso o un fodero per le biciclette che si ficcano il naso nel punto in cui non lo fa t appartenere.

Non puoi semplicemente suggerire una Style Guide?

In effetti, questo sembra essere quello che vuoi fare: credi nel valore delle guide di stile, tieni alta la coerenza e vuoi evangelize per la dedica alle linee guida di stile unificate.

Va bene, a patto che tu sia veramente chiaro che questo è ciò che stai facendo e ciò che vuoi ottenere. Se credi in una particolare guida di stile e credi che sia la guida di The One True Style, o almeno meglio di qualsiasi cosa che i pagani senza legge stanno facendo pratica, allora va bene anche questo.

Ma a ciò che la gente non apprezza viene detto che il loro comportamento non è conforme a regole non ufficiali, non vincolanti e in gran parte arbitrarie provenienti da una fonte che non considerano un'autorità legittima. Quando è quello che hai deciso di fare, o se semplicemente questo è ciò che sei percepito da fare, riceverai meno "trattamento da tappeto rosso", e più del trattamento "nativi arrabbiati con lance e un grande piatto di ebollizione".

    
risposta data 03.02.2014 - 21:40
fonte
3

Per offrire un'opinione alternativa qui, in molti casi, un tale cambiamento sarebbe benvenuto. I progetti open source tendono ad avere molti autori. Spesso non esiste uno "stile di codifica"; lo stile è proprio quello che è successo alla persona che ha scritto il codice in questione. Se un file è stato scritto da una persona diversa da un'altra, lo stile potrebbe essere diverso. Anche nei progetti in cui esiste uno stile di consenso, a meno che non controllino regolarmente questo stile, spesso non viene utilizzato.

Un esempio comune di questo nella mia esperienza è quando qualcuno contribuisce con del codice tramite una richiesta pull di qualità relativamente bassa in termini di stile. Tuttavia, il codice potrebbe funzionare. Persone diverse hanno opinioni diverse su questo. Alcune persone si rifiuteranno di unire una richiesta di pull a meno che lo stile sia buono. Ad alcune persone non importa, a patto che il codice funzioni. Alcune persone preferiscono avere un buon stile, ma non vogliono spaventare i contributori con una serie di commenti "aggiusta i bianchi qui" (personalmente mi sento sempre un po 'colpevole di fare questi commenti, anche se so che sono per il meglio bene del codebase, perché sembra che potrebbe spaventare il contributore di distanza).

Quindi non dare per scontato che lo stile che vedi sia lo stile che il progetto vuole. In effetti, questo può probabilmente essere generalizzato per contribuire all'open source in generale: non dare per scontato che il codice di un progetto open source sia il codice che vuole .

Tuttavia, dovresti essere consapevole di alcune cose:

  • Alcune persone sono religiose riguardo allo stile. Se diventa chiaro che non vogliono muoversi, allora non preoccuparti.

  • Questo è un problema enorme bikehed . Tutti e il loro fratello hanno un'opinione su queste cose. Per questo motivo ottenere una tale richiesta di pull può essere difficile.

  • Può anche essere difficile ottenere una tale richiesta di pull unita perché otterrà conflitti di fusione molto rapidamente; in pratica ogni volta che qualsiasi parte della base di codice che hai modificato cambia, anche se è in modi banali.

Vorrei attenermi all'approccio "chiedi prima". Se sono aperti a questo, e sei disposto a seguire la richiesta di pull fino al completamento, allora vai a prenderlo.

    
risposta data 03.02.2014 - 03:46
fonte
2

Ero abituato ad avere una buona guida di stile, ma dato lo stato delle cose in Ruby, "Mi sono trasferito".

Fondamentalmente vivo con ciò con cui lavoro e in ogni caso seguo le convenzioni generali che ho imparato da un certo numero di lavori.

Per Ruby, che è il mio linguaggio di scelta, ho anche (nella mia testa) diviso lo stile in universalmente accettato, generalmente accettato, le mie preferenze e le mie migliori pratiche. Cose che sono universalmente accettate Potrei inviare una modifica come parte di una richiesta di modifica per un problema o una richiesta di ramo di funzionalità.

Esempi di ogni stile (secondo me):

Universalmente accettato per Ruby:

  • spazi non schede
  • due rientri dello spazio
  • method_names_use_underscores
  • Le costanti iniziano con Caps.

Generalmente accettato:

  • Non utilizzare le parentesi se non necessario
  • Utilizza { } per i blocchi di una riga e do end per i blocchi a più linee.
  • CONSTANTS_ARE_IN_ALL_CAPS
  • Utilizza istruzioni predicative ( cond ? true : false ) su if then se l'espressione si adatta su 1 riga.

Preferenze personali:

  • Lunghezza della linea di 120 caratteri
  • Le istruzioni
  • case when sono indentate 2 dall'istruzione case.
  • Utilizza le virgolette singole a meno che non sia necessario il doppio.

Nessun accordo:

  • Case Statement
  • Lunghezza della linea

Best practice:

  • piccoli metodi, < = 5 righe, se possibile.
  • classi piccole, < = 100 righe, se possibile.
  • Evita costanti
  • Il codice ha dei test

Infine, come altri hanno dettagliato, si dovrebbe chiedere prima. Alla fine della giornata la chiave per lo stile è la comunicazione tra gli sviluppatori e la sensibilità per gli altri. Ad esempio, se voglio apportare una modifica di stile a un progetto open source, eseguirò spesso una richiesta di pull per una o due effettive funzionalità o correzioni di errori. Una volta che il maintainer mi conosce e vede che ho contribuito, allora potrei suggerire cambiamenti di stile. Comunque li suggerisco . Ad esempio "Mentre facevo un'altra funzione sul progetto x, ho notato che hai 4 rientri nello spazio in un paio di file e mi chiedevo se potevo cambiarli in 2?"

    
risposta data 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?

In breve, no!

Naturalmente, presumo qui che queste siano solo questioni di stile e non siano veri bug - le richieste di pull per quest'ultimo sarebbero sempre sensate (IMO). Sto anche assumendo che tu sia estraneo a il progetto e non in contatto con le persone che lo mantengono già.

Tuttavia, tra tutte le lingue ci sono sempre alcune persone che hanno le loro preferenze che si differenziano dalla guida allo stile, nel bene o nel male, e cercano di applicare quelle modifiche di stile alle persone che non conosci e un progetto che non fai parte di se nient'altro potrebbe sembrare un po 'scortese. Dopo tutto, cosa stai ottenendo (in realtà) se la richiesta è stata accettata? Con ogni probabilità, se i membri di un progetto volevano cambiare lo stile in qualcosa di diverso, lo avrebbero già fatto - e tutto quello che faresti con quella richiesta è forzare uno stile su di loro che non necessariamente funziona meglio per i membri esistenti.

Questo cambia leggermente se sei disposto a contribuire al progetto in altri modi, oltre a fare "correzioni" di stile. Direi che se vuoi aprire un dialogo con i manutentori su come ti piacerebbe lavorare sul progetto, ma trovi uno stile non standard difficile, allora va bene. Ma in realtà non andrei in giro a creare ciecamente richieste di pull per un gruppo di progetti che contengono solo cambiamenti di stile!

    
risposta data 03.02.2014 - 02:30
fonte
1

QUALSIASI modifica può introdurre bug, anche cambiando la formattazione tipografica del codice.
Pertanto non dovrebbero essere apportate modifiche al codice a meno che non ci sia un business case valido, e "ma il codice non sembra carino" o "il codice non segue gli standard di codifica" non è un caso aziendale valido. Il rischio è semplicemente troppo grande.
Ora, se stai apportando importanti modifiche a un file sorgente, è possibile che l'intero file in linea con gli standard sia accettabile, ma molto probabilmente si apportano piccole modifiche, nel qual caso è quasi universalmente preferibile mantenere le proprie modifiche in linea con il codice esistente anche se quel codice esistente non segue gli standard di codifica.
Diamine, il codice potrebbe essere il risultato di un generatore di codice e, a volte, essere rigenerato. I generatori di codice sono noti per produrre codice brutto ...

    
risposta data 03.02.2014 - 15:41
fonte
1

Ho un paio di progetti .NET di discreto successo, e ho avuto un paio di PR di persone che sembrano aver passato il codice con ReSharper e StyleCop e "sistemato" un sacco di cose. Non accetto i PR, per un paio di motivi:

  • Anche se molti dei cambiamenti sono in meglio, alcuni di essi hanno un impatto negativo sul codice in qualche modo, di solito le prestazioni e la selezione delle parti buone è un lavoro troppo duro.
  • Anche se tutti i cambiamenti fossero positivi o addirittura vantaggiosi, ogni modifica di ogni file in ogni commit deve essere rivista e, ancora, ci vuole troppo tempo.

Detto questo, se qualcuno volesse aggiungere un controllo degli errori o dei commenti dei documenti migliori, accetterei il PR in un batter d'occhio.

    
risposta data 02.03.2014 - 02:06
fonte
-1

Devi scoprire se i manutentori considerano tali violazioni di stile di codifica un difetto o se il progetto ha uno standard diverso per lo stile di codifica.

Se ha uno standard diverso, puoi offrirti di aiutare a documentarlo o formalizzarlo. Quindi potrebbe risultare che vi sono alcune violazioni di quello stile e potresti correggerle.

    
risposta data 03.02.2014 - 09:38
fonte

Leggi altre domande sui tag