Skip to content

Fix mail template error #29410

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 26, 2024
Merged

Conversation

wxiaoguang
Copy link
Contributor

@wxiaoguang wxiaoguang commented Feb 26, 2024

A mistake by #29392

@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Feb 26, 2024
@wxiaoguang wxiaoguang added this to the 1.22.0 milestone Feb 26, 2024
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 26, 2024
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 26, 2024
log.Error("Failed to parse template [%s/body]: %v", name, err)
if !setting.IsProd {
log.Fatal("Please fix the mail template error")
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm of the opinion that behaviour differences between RUN_MODE should be kept minimal. Is it warranted here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think it should also "Fatal" (just like web page HTML templates)

But the old code just reports the error message and continues. So I chose to keep the old behavior at the moment.

So I think it is warranted that "the behavior is the same as the old code and the change is minimal", but it doesn't warrant that "the behavior is the same for production/dev"

Feel free to change the behavior.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 26, 2024
@techknowlogick techknowlogick enabled auto-merge (squash) February 26, 2024 22:05
@techknowlogick techknowlogick merged commit eb2fc18 into go-gitea:main Feb 26, 2024
zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 27, 2024
* giteaofficial/main:
  Fix mail template error (go-gitea#29410)
  Document our issue locking policy (go-gitea#29433)
  Fix htmx rendering the login page in frame on session logout (go-gitea#29405)
  Ignore empty repo for CreateRepository in action notifier (go-gitea#29416)
  Fix incorrect tree path value for patch editor (go-gitea#29377)
  Fix logic error from go-gitea#28138 (go-gitea#29417)
@wxiaoguang wxiaoguang deleted the fix-mail-tmpl branch March 1, 2024 13:28
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants