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.

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

Should also support upper-case README files #20581

Merged
merged 2 commits into from
Aug 1, 2022
Merged

Conversation

BLumia
Copy link
Member

@BLumia BLumia commented Aug 1, 2022

Address the issue introduced in #20508 (79741dd), also added some case in existing test that cover this issue.

IsReadmeFileExtension only match the lower-case readme file, and also require the extensions use the same caption case as the provided ones, so README.md and readme.MD will no longer be supported. After this patch, it will be once again become supported.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Aug 1, 2022
@lunny lunny added type/bug skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Aug 1, 2022
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Although I think it's not necessary to make the ext name case-insensitive, it's not bad to do so.

@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 Aug 1, 2022
@BLumia
Copy link
Member Author

BLumia commented Aug 1, 2022

Although I think it's not necessary to make the ext name case-insensitive, it's not bad to do so.

For the record, it's for something like ".zh_CN.md" so "README.zh_cn.md" will also be okay. Also maybe some DOS fans will also find it be useful so "README.MD" can be accepted :)

It's not required in the current code base through since localizedExtensions() will always produce lower-cased language code as extensions :)

@lunny
Copy link
Member

lunny commented Aug 1, 2022

make L-G-T-M work

@lunny lunny merged commit 72b1fd7 into go-gitea:main Aug 1, 2022
@BLumia BLumia deleted the fix-reg-20508 branch August 1, 2022 13:30
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 2, 2022
* giteaofficial/main:
  Rework mailer settings (go-gitea#18982)
  Add default value for clone URLs (go-gitea#20600)
  [skip ci] Updated translations via Crowdin
  docs: zh-cn translations for fail2ban setup (go-gitea#20588)
  Should also support upper-case README files (go-gitea#20581)
  Fix typos in backup documentation (go-gitea#20567)
vsysoev pushed a commit to IntegraSDL/gitea that referenced this pull request Aug 10, 2022
Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
@wxiaoguang wxiaoguang added this to the 1.18.0 milestone Oct 7, 2022
@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.
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. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants