Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix wrong default value for update checker on app.example.ini #22084

Merged
merged 3 commits into from Dec 11, 2022
Merged

Fix wrong default value for update checker on app.example.ini #22084

merged 3 commits into from Dec 11, 2022

Conversation

lunny
Copy link
Member

@lunny lunny commented Dec 10, 2022 •
edited by jolheiser

Fix #22078

Note from @jolheiser: this is not breaking, but deserves a note in the changelog/blog to let users know. 🙂
The label can be removed after the note has been added.

delvh
delvh reviewed Dec 10, 2022
View reviewed changes
Copy link
Member

@delvh delvh left a comment •
edited

Choose a reason for hiding this comment

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

  1. cheatsheet.en_us also has the incorrect value
  2. shouldn't we mark that PR breaking for anyone who assumed that the opposite is the case?
    Even if it isn't actually breaking itself, it's still good to know that it is different than what you thought…

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 10, 2022
@yardenshoham yardenshoham added this to the 1.19.0 milestone Dec 11, 2022
@lunny lunny added the type/docs This PR mainly updates/creates documentation label Dec 11, 2022
@lunny
Copy link
Member Author

lunny commented Dec 11, 2022

  1. cheatsheet.en_us also has the incorrect value
  2. shouldn't we mark that PR breaking for anyone who assumed that the opposite is the case?
    Even if it isn't actually breaking itself, it's still good to know that it is different than what you thought…
  1. Done.
  2. I don't think this PR is a break change. It just corrected the document which is inconsistent with code.

@delvh
Copy link
Member

delvh commented Dec 11, 2022

Yes, I also don't think it is a breaking change.
However, we should notify users somehow that the default behavior has changed so that they can disable it if necessary.

@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 Dec 11, 2022
@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 Dec 11, 2022
@jolheiser jolheiser merged commit c9ef03a into go-gitea:main Dec 11, 2022
@jolheiser jolheiser added the pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! label Dec 11, 2022
@jolheiser
Copy link
Member

I've marked this as breaking only so we remember to make a note about it when generating the changelog/blog.

@lunny lunny deleted the lunny/fix_update_checker_doc branch December 12, 2022 02:08
zjjhot added a commit to zjjhot/gitea that referenced this pull request Dec 12, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
8cb75e3 
* giteaofficial/main:
  Use multi reader instead to concat strings (go-gitea#22099)
  Fix sorting admin user list by last login (go-gitea#22081)
  Fix wrong default value for update checker on app.example.ini (go-gitea#22084)
  fix(config): remove context on config template (go-gitea#22096)
  [skip ci] Updated licenses and gitignores
  Update xorm (go-gitea#22094)
  Remove unnecessary whitespace in snapcraft.yaml (go-gitea#22090)
  Rename almost all Ctx functions (go-gitea#22071)
  Change ID pattern of raw content container for issue (go-gitea#21966)
  Optimize html templates (go-gitea#22080)
  Add API management for issue/pull and comment attachments (go-gitea#21783)
  Rename actions to operations on UI (go-gitea#22067)
  Update go dev dependencies (go-gitea#22064)
@AbdulrhmnGhanem AbdulrhmnGhanem mentioned this pull request Dec 30, 2022
Merged
@kirkryan kirkryan mentioned this pull request Apr 19, 2023
Closed
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jolheiser jolheiser jolheiser approved these changes

@delvh delvh delvh approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. pr/breaking Merging this PR means builds will break. Needs a description what exactly breaks, and how to fix it! type/docs This PR mainly updates/creates documentation type/miscellaneous
Projects
None yet
Milestone
1.19.0
Development

Successfully merging this pull request may close these issues.

Update checker documentation say it is disabled by default but it is enabled
5 participants
@lunny @delvh @jolheiser @yardenshoham @GiteaBot