Jump to conversation
Unresolved conversations (0)
Nice work!

Nice work!

All of your conversations have been resolved.

Resolved conversations (17)
@Gusted Gusted Jul 29, 2022
Yes interesting sentence. This need some extra words I assume? CC @wxiaoguang https://github.com/go-gitea/gitea/commit/466d6ba3f576ab8b04aa33f6eee488c1131de9f5
Outdated
custom/conf/app.example.ini
wxiaoguang
wxiaoguang
@wxiaoguang wxiaoguang Jul 28, 2022
```suggestion ;; you can set SMTP_PORT instead and this will be inferred. ;; Since 1.18 ;PROTOCOL = ```
Outdated
custom/conf/app.example.ini
@lunny lunny Jul 28, 2022
Maybe we need break notice here.
Outdated
.../doc/advanced/config-cheat-sheet.en-us.md
wxiaoguang
wxiaoguang
@wxiaoguang wxiaoguang Jul 14, 2022
Should the install form be updated accordingly? https://github.com/clarfonthey/gitea/blob/mailer-settings/templates/install.tmpl
routers/install/install.go
wxiaoguang clarfonthey
wxiaoguang and Clar Fon
@lunny lunny Jun 5, 2022
The logic should be if `SMTPAddr` is not set but `HOST` set, then fallback.
modules/setting/mailer.go
@lunny lunny Jun 5, 2022
The logic should be if `SMTPAddr` is not set but `HOST` set, then fallback.
modules/setting/mailer.go
@lunny lunny Jun 5, 2022
Old setting should be kept some versions and marked as deprecated.
custom/conf/app.example.ini
@lunny lunny Jun 5, 2022
Old setting should be kept some versions and marked as deprecated.
custom/conf/app.example.ini
@Gusted Gusted May 8, 2022
This error doesn't sound user-friendly.
Outdated
modules/setting/mailer.go
clarfonthey Gusted
Clar Fon and Gusted
@Gusted Gusted May 3, 2022
Could we use [`net.(IP).IsLoopback()`](https://pkg.go.dev/net#IP.IsLoopback) here?
Outdated
modules/setting/mailer.go
Gusted clarfonthey
Gusted and Clar Fon
@wxiaoguang wxiaoguang Apr 1, 2022
Generally L-G-T-M. I prefer to use a URL to set all these protocol/addr/port. eg: ``` SMTP_ADDR = smtp+starttls://username:password@smtp.gmail.com:587 ``` And IIRC there are some SMTP options on the installation page. Personally I prefer to remove these options from the installation page, but I don't know how others think about it .....
Outdated
custom/conf/app.example.ini
clarfonthey
Clar Fon
@Gusted Gusted Mar 2, 2022
```suggestion host := "localhost" if opts.Protocol == "smtp+unix" { host = opts.SmtpAddr } ```
Outdated
services/mailer/mailer.go
clarfonthey
Clar Fon
@Gusted Gusted Mar 2, 2022
```suggestion return fmt.Errorf("could not load provided client certificates: %v", err) ```
Outdated
services/mailer/mailer.go
clarfonthey
Clar Fon
@Gusted Gusted Mar 2, 2022
A NPE can happen here if a user does have client cert specified but not the correct protocol(should probably be warned about in setting/mailer.go)
services/mailer/mailer.go
Gusted clarfonthey
Gusted and Clar Fon
@Gusted Gusted Mar 2, 2022
`HELO` is now by default disabled? ```suggestion EnableHelo: sec.Key("ENABLE_HELO").MustBool(true), ```
Outdated
modules/setting/mailer.go
clarfonthey
Clar Fon
@Gusted Gusted Mar 2, 2022
> If you don't know what this is, leave it blank. I think this should be left out. See relevant discussion: https://github.com/go-gitea/gitea/pull/17440#discussion_r752890770 Also could the default value be added for this key?
Outdated
custom/conf/app.example.ini
@Gusted Gusted Mar 2, 2022
Multi line comments are preferred 😅
Outdated
custom/conf/app.example.ini
clarfonthey
Clar Fon