postgres-storage icon indicating copy to clipboard operation
postgres-storage copied to clipboard

Write better code

Open yroc92 opened this issue 4 years ago • 0 comments

From @mholt

https://github.com/yroc92/postgres-storage/blob/master/module.go <-- could remove this file

module.go package certmagicsql

https://github.com/yroc92/postgres-storage|yroc92/postgres-storageyroc92/postgres-storage | Added by GitHub

Matt Holt 1 month ago https://github.com/yroc92/postgres-storage/blob/master/caddy.go#L20-L21 Remove the pointer receiver, I think

caddy.go func (s *PostgresStorage) CertMagicStorage() (certmagic.Storage, error) { return NewStorage(*s) https://github.com/yroc92/postgres-storage|yroc92/postgres-storageyroc92/postgres-storage | Added by GitHub

Matt Holt 1 month ago What's this for? https://github.com/yroc92/postgres-storage/blob/master/.go-version

.go-version 1.16.0 https://github.com/yroc92/postgres-storage|yroc92/postgres-storageyroc92/postgres-storage | Added by GitHub

Matt Holt 1 month ago Where do you use this interface? https://github.com/yroc92/postgres-storage/blob/master/storage.go#L18-L24

storage.go // DB represents the required database API. You can use a *database/sql.DB. type DB interface { BeginTx(context.Context, *sql.TxOptions) (*sql.Tx, error) ExecContext(context.Context, string, ...interface{}) (sql.Result, error) QueryRowContext(context.Context, string, ...interface{}) *sql.Row Show more https://github.com/yroc92/postgres-storage|yroc92/postgres-storageyroc92/postgres-storage | Added by GitHub

Matt Holt 1 month ago (These questions are curiosities, not criticisms)

Matt Holt 1 month ago https://github.com/yroc92/postgres-storage/blob/master/storage.go#L193 Definitely use prepared statements, not formatted strings :grimacing:

storage.go rows, err := s.Database.QueryContext(ctx, fmt.Sprintf(select key from certmagic_data where key like '%s%%', prefix)) https://github.com/yroc92/postgres-storage|yroc92/postgres-storageyroc92/postgres-storage | Added by GitHub

Matt Holt 1 month ago This should probably not return an error, but instead should wait as long as it needs to obtain the lock: https://github.com/yroc92/postgres-storage/blob/master/storage.go#L103

storage.go return err https://github.com/yroc92/postgres-storage|yroc92/postgres-storageyroc92/postgres-storage | Added by GitHub

yroc92 avatar Nov 03 '21 03:11 yroc92