Zeitwerk è un caricatore di codice efficiente e thread-safe per Ruby. Finché il progetto segue le convenzioni di denominazione, Zeitwerk può individuare i file corretti e caricarli su richiesta o in anticipo, senza la necessità di alcun require o require_dependency. Inoltre, secondo quell’articolo https://weblog.rubyonrails.org/2019/2/22/zeitwerk-integration-in-rails-6-beta-2/, potrebbe offrire un piccolo incremento delle prestazioni all’applicazione.
Ci sono alcune operazioni che devo eseguire per farlo funzionare:
Cambiare il nome di alcune classi per seguire la convenzione di denominazione di Rails. Ad esempio, il file canonical_url.rb dovrebbe definire la classe CanonicalUrl invece di CanonicalURL. Allo stesso modo, il file ondiff.rb dovrebbe definire la classe Onpdiff invece di ONPDiff. Un approccio alternativo sarebbe collegare un inflector personalizzato al progetto; tuttavia, penso che seguire la convenzione possa essere una scelta migliore - GitHub - fxn/zeitwerk: Efficient and thread-safe code loader for Ruby · GitHub
Analogamente al punto precedente, per convenzione le validazioni personalizzate presenti nella directory validations dovrebbero essere racchiuse nel modulo Validations. Inoltre, alcune validazioni ereditano da EachValidator e dovrebbero essere accessibili senza namespace. Intendo spostarle in una directory separata e aggiungerle ai percorsi di autoload.
Rimuovere tutti i require_dependency e assicurarsi che il progetto funzioni correttamente.
Rimuovere tutti i require e assicurarsi che Discourse funzioni correttamente.
Assicurarsi che tutti i plugin possano accedere alle dipendenze richieste. Non so ancora come realizzare ciò. Vorrei prima far funzionare Discourse senza alcun plugin.
Naturalmente, ci sono ancora molte incognite da risolvere. Terrò aggiornati sui progressi; tuttavia, se siete interessati a dare un’occhiata, ho iniziato a sperimentare qui: Commits · KrisKotlarek/discourse · GitHub
Fatemmi sapere se notate eventuali svantaggi nell’implementazione di Zeitwerk o se pensate che abbia tralasciato qualcosa.
Ho fatto alcuni progressi con Zeitwerk, ma ho cambiato approccio. Il mio piano iniziale era modificare ogni luogo in cui Discourse non segue la convenzione di denominazione dei file di Zeitwerk. Dopo alcune correzioni, ho capito che questo era solo la punta dell’iceberg e ho notato che, se avessi seguito quella strada, la pull request sarebbe stata difficile da leggere e da fondere con sicurezza nel ramo master. Ad esempio, tutte le classi di job sotto una directory regolare dovrebbero avere lo spazio dei nomi Regular, lo stesso per Onceoff e Scheduled.
Ho deciso di fare un passo indietro e pensare a un approccio più evolutivo che rivoluzionario.
Ho deciso che sarebbe stato meglio introdurre un Inflector personalizzato, che copra tutti i file che non seguono la convenzione di Zeitwerk. Il vantaggio principale è che potremmo distribuire questa piccola modifica e, una volta soddisfatti di Zeitwerk e senza alcuna riduzione delle prestazioni, possiamo iniziare a correggere la convenzione file per file in pull request ragionevolmente piccole.
Ho trovato alcuni problemi che non potevano essere risolti con un Inflector personalizzato, quindi ho apportato ulteriori correzioni per farlo funzionare.
La pull request è ancora in corso, ma a questo stadio posso eseguire Discourse con Zeitwerk e i plugin predefiniti, eseguire tutti gli spec e i benchmark senza problemi.
Volevo prima raggiungere uno stato stabile in cui tutti gli spec passano. Ora posso iniziare con sicurezza a rimuovere tutti i require_dependency uno per uno e testare anche i plugin ufficiali. Una volta che tutto sarà pronto, condividerò con voi i risultati dei benchmark in questo post.
Per far funzionare i plugin ho dovuto creare alcuni piccoli pull request. Una volta che saranno uniti, penso che i test su Discourse dovrebbero passare.
Possiamo provare un altro metodo per il benchmark. Cosa ne pensi di più iterazioni, qualcosa che duri un’ora? Inoltre, invece di prendere il miglior risultato, confrontiamo la media di ogni esperimento. Questo potrebbe fornire numeri più coerenti. Che ne dici?
Nei miei pull request per i plugin, noterai che molte correzioni riguardano la ricerca nello spazio dei nomi globale.
Ho modificato codice come:
module ::Jobs
class TranslatorMigrateToAzurePortal < Jobs::Onceoff
in:
module ::Jobs
class TranslatorMigrateToAzurePortal < ::Jobs::Onceoff
Una cosa mi ha dato da pensare riguardo a quella soluzione: perché funzionava prima di Zeitwerk? Quella domanda, quando qualcosa funziona ma non dovrebbe, è sempre delicata
Zeitwerk è più rigoroso. Una volta ho provato a caricare il codice prima della correzione e si lamentava che Jobs::Jobs::Onceoff non poteva essere trovato.
Sam ha suggerito nel pull request FIX: Use top-level namespace for base classes · discourse/discourse-prometheus-alert-receiver@ef9c238 · GitHub che, invece di usare < ::Jobs::Onceoff, possiamo semplicemente usare < Onceoff, e ha ragione. Ho verificato che senza lo spazio dei nomi funziona ugualmente. Penso che usare :: significhi esplicitare che ereditiamo da una classe del core di Discourse, tuttavia possiamo scegliere entrambe le strade.
Credo che quando il codice è così ravvicinato, questa lettura sia ottima:
module ::Jobs
class TranslatorMigrateToAzurePortal < Onceoff
Se il codice inizia a distanziarsi, ha più senso essere espliciti… ad esempio:
module ::Jobs
[ 50 righe omesse]
class TranslatorMigrateToAzurePortal < ::Jobs::Onceoff
Detto questo, sono indeciso, quindi sono a posto in entrambi i casi. ::Jobs::Onceoff è abbastanza breve e super esplicito, quindi possiamo semplicemente procedere così per ora.
Mi piacerebbe molto iniziare a far fondere questa modifica. @kris.kotlarek, ora è in uno stato pronto per la fusione? Il tempismo è ottimo perché abbiamo appena avuto una beta.
@sam Penso che siamo pronti. Ho fatto il rebase con l’ultimo master e ho apportato una piccola modifica a Webauthn.
Ho verificato tre cose:
ho avviato il server in locale e ho cliccato un po’ per assicurarmi che funzionasse come previsto
ho eseguito gli spec
ho scaricato tutti i plugin ufficiali e ho verificato che gli spec per i plugin passino (dobbiamo prima unire le modifiche per i plugin)
