config: validate and print warnings for deprecated options
Describe the pull request
close: https://github.com/gogs/gogs/issues/6498
A clear and concise description of what the pull request is about, i.e. what problem should be fixed?
Link to the issue:
Checklist
- [x] I agree to follow the Code of Conduct by submitting this pull request.
- [x] I have read and acknowledge the Contributing guide.
- [x] I have added test cases to cover the new code or have provided the test plan.
Test plan
Thanks for the PR!
However, it looks incomplete and missing tests, could you finish it before requesting review?
Hi @unknwon , yes. Noted, next time, will set a draft PR before asking for the review. Btw, I'm still creating the unit test. But I found some problems, I'm not sure how to test/assert the log.Warn :thinking: . I'm thinking of returning some warnMessages []string for the warnDeprecated function, so it can be asserted.
'm thinking of returning some
warnMessages []stringfor thewarnDeprecatedfunction, so it can be asserted.
Yep, that's what I would do and recommend 👍
I do have more general question about whether we need this PR at all though. The tip of the main is already 0.13.0 that no longer reads those config options, so they're not just deprecated but deleted.
Let's say we do agree it's a QoL improvement, the logic needs to be version aware because for example, [service] section could be repurposed, to have, service-related settings. Or re-introduce the same section-key pair for a different purpose.
All that to say, adding a comment saying something like "Delete this function after 0.14.0 is released." would be good enough.
Let's say we do agree it's a QoL improvement, the logic needs to be version aware because for example, [service] section could be repurposed, to have, service-related settings. Or re-introduce the same section-key pair for a different purpose.
Oh, okay, it makes sense. Since it will be deleted in next version and can be repurpose later.
All that to say, adding a comment saying something like "Delete this function after 0.14.0 is released." would be good enough
I see, okay, I'll close this PR.
All that to say, adding a comment saying something like "Delete this function after 0.14.0 is released." would be good enough
I see, okay, I'll close this PR.
Hmm, sorry if I was misleading... didn't mean to close this PR. I think what's in the current diff still makes sense to the purpose of this PR.
i.e. just do
// warnDeprecated warns about deprecated configuration sections and options.
+//
+// NOTE: Delete this function after 0.14.0 is released.
func warnDeprecated(cfg *ini.File) []string {
Then finish your TODOs and add tests. Sounds right?