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

resolves #1231 sort README files #1233

Merged
merged 1 commit into from
Mar 31, 2019

Conversation

mojavelinux
Copy link
Contributor

Description

The main reason for this change is to choose the README that users most expect to be chosen. Currently, there isn't really any rhyme or reason to which README is selected because the files aren't explicitly sorted. With this change, README will be selected over README.md, and README.md will be selected over README-zh_CN.md (or README.zh_CN.md).

The changes included are:

  • sort README files, first by file extension, then by basename
  • prefer README basename over all other files
  • prefer file without extension over file with extension

Completed Tasks

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I wrote tests and ran bundle exec rake locally (if code is attached to PR).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 93.74% when pulling 3915182 on mojavelinux:issue-1231-readme-sorting into 6e7be09 on lsegal:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 93.74% when pulling 3915182 on mojavelinux:issue-1231-readme-sorting into 6e7be09 on lsegal:master.

@coveralls
Copy link

coveralls commented Mar 30, 2019

Coverage Status

Coverage decreased (-0.05%) to 93.68% when pulling 88da7e6 on mojavelinux:issue-1231-readme-sorting into 87ef6d8 on lsegal:master.

@skalee
Copy link
Contributor

skalee commented Mar 30, 2019

Tests should succeed after merging #1234 in.

lib/yard/cli/yardoc.rb Outdated Show resolved Hide resolved
lib/yard/cli/yardoc.rb Outdated Show resolved Hide resolved
@mojavelinux
Copy link
Contributor Author

PR updated as requested.

@mojavelinux
Copy link
Contributor Author

Build is green.

lib/yard/cli/yardoc.rb Outdated Show resolved Hide resolved
@mojavelinux
Copy link
Contributor Author

mojavelinux commented Mar 31, 2019 via email

- sort README files, first by file extension, then by basename
- prefer README basename over all other files
- prefer file without extension over file with extension
- don't glob for files if README is missing and files list is empty
@lsegal lsegal merged commit 5a96ccd into lsegal:master Mar 31, 2019
@lsegal
Copy link
Owner

lsegal commented Mar 31, 2019

Thanks for working on this!

@mojavelinux
Copy link
Contributor Author

You're welcome. Glad to help!

@mojavelinux mojavelinux deleted the issue-1231-readme-sorting branch March 31, 2019 23:12
@lsegal
Copy link
Owner

lsegal commented Apr 1, 2019

Seems as though there are some failures on Windows due to this logic, probably conflicting with Dir.glob on other platforms. I'm looking into this now.

lsegal added a commit that referenced this pull request Apr 1, 2019
@mojavelinux
Copy link
Contributor Author

That's very unexpected. I'll be interested to hear what you find.

lsegal added a commit that referenced this pull request Apr 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants