-
Notifications
You must be signed in to change notification settings - Fork 701
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
cabal-install: check: Whitelist doc file extensions #8747
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think extensions should include .TXT
and maybe more generally cater for case-insensitive file systems.
isDesirableExtraDocFile paths path = map toLower basename `elem` paths | ||
isDesirableExtraDocFile :: ([FilePath], [FilePath]) -> FilePath -> Bool | ||
isDesirableExtraDocFile (basenames, extensions) path = | ||
map toLower basename `elem` basenames && ext `elem` extensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ext
is not subjected to map toLower
here. I wonder how this plays out in case-insensitive filesystems (e.g. on Windows).
It seems that some folks call the file "CHANGELOG.TXT", e.g. https://github.com/moodle/moodle/blob/0780e87f06eb834786f562316ad9699480bcd24e/lib/tcpdf/CHANGELOG.TXT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed and added test.
It is worth to update hackage-server as well (see haskell/hackage-server#1179).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
One more thing in the test seems to still require an update. Also please rebase to take advantage of the newest CI fix. |
Rebased and (hopefully) fixed. File order does not seem to be deterministic or is platform-dependent. As we have two similar test cases, I fixed it by keeping only one file in each case. |
Am I correct this needs a backport to 3.10? |
Yes only if you backported #8657. |
I did not. But it was merged before 3.10 has been cut. Wasn't it? |
@mergify backport 3.10 |
✅ Backports have been created
|
* check: Test only doc files with supported extensions * Cleanup --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit bae536f)
cabal-install: check: Whitelist doc file extensions (backport #8747)
Fixes #8746
Please also shortly describe how you tested your change. Bonus points for added tests!