make sure that README files in markup / text take preference over any other README files with random suffix Fixes https://pagure.io/pagure/issue/2758 Signed-off-by: Karsten Hopp karsten@redhat.com
1 new commit added
We could shortcut the following lines if there is only one README file
Do you mean like this ? if len(readmes) > 1: for i in order: for j in readmes: if i == j.name: return j elif len(readmes) == 1: return readmes[0] return None
Something like this yes
I wonder if we couldn't do something like:
readme_names = [j.name for j in readmes] return readmes[min([order.index(f) for f in readme_names])]
(untested)
Finally, it would be cool to add some tests for this.
Other than that, it looks great, thanks! :)
I'll test your version
This fails when there's a readme that's not in the order list. order.index('README.notinlist') ValueError: 'README.notinlist' is not in list
What is we use:
readmes[min([order.index(f) for f in readme_names if f in order])]
?
Your version picks README.Propaganda for the https://src.fedoraproject.org/rpms/desktop-backgrounds use-case.
The lists readmes and order are not in sync, so I suspect you meant
readmes[min([readme_names.index(f) for f in readme_names if f in order])]
But that still fails when there's only a README available that is not in the order list:
ValueError: min() arg is an empty sequence
I'd like to keep it 'as is' for now as I've tested my version with all combinations I could think of and concentrate on adding tests so that any code changes can be easily verified
Alright, thanks for double-checking and +1 for the tests :)
rebased onto 40183bde115c56d800092d3969dcdd680d1269f9
Could you rebase rather than merging?
rebased onto dd024fdaf5e3cb4aa62a8ca2f69dae6c96e3a11f
tests are all passing, thanks! :)
Pull-Request has been merged by pingou
make sure that README files in markup / text take preference over any other README files with random suffix
Fixes https://pagure.io/pagure/issue/2758
Signed-off-by: Karsten Hopp karsten@redhat.com