On Sat, 2018-07-21 at 10:37:43 +0100, Chris Lamb wrote: > tags 872611 - moreinfo > tags 872611 + patch > forwarded 872611 https://salsa.debian.org/lintian/lintian/merge_requests/12 > thanks
> > Sorry had forgotten about this one. Not sure which part do you find > > unsafe? The grep was just an example, I'd probably use a regex similar > > to the one I used on codesearch.debian.net, so something like: > > > > m/(?:select-editor|sensible-(?:pager|editor|browser))/ > > Thanks! Can you give this a quick review? > > https://salsa.debian.org/lintian/lintian/merge_requests/12 Had to check locally as salsa was not willing to show that page. :/ Ok some comments: - On the description you might want to mention Recommends in addition to Depends. - It might make sense to consider also Suggests, in case the packages conditionally use these tools only when present. - You mention checking all "source files" in several places, but I think you mean files in the binary packages? - The space removal in find_cruft() seems suspect. :) - The new argument in full_text_check() seems too specialized, dunno, perhaps pass it as a code block instead of a bool, so that the function keeps being generic? - I think _has_sensible_utils() might have lost the self-dependency handling special-case from the original code? - I still find the one-line-subject-as-a-commit-message-pattern-including-bug-closure-and-rationale-and-everything-else-that-might-come-by quite straining. ;) Thanks, Guillem
