DescriptionCheck: catch ending with a full stop#472
DescriptionCheck: catch ending with a full stop#472arthurzam wants to merge 3 commits intopkgcore:masterfrom
Conversation
Codecov ReportBase: 81.09% // Head: 81.09% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #472 +/- ##
========================================
Coverage 81.09% 81.09%
========================================
Files 55 55
Lines 7923 7925 +2
Branches 1474 1775 +301
========================================
+ Hits 6425 6427 +2
Misses 1400 1400
Partials 98 98
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| yield BadDescription("generic eclass defined description", pkg_desc=desc, pkg=pkg) | ||
| elif s in (pkg.package.lower(), pkg.key.lower()): | ||
| yield BadDescription("generic package description", pkg_desc=desc, pkg=pkg) | ||
| elif desc.endswith(tuple(".,:;")): |
There was a problem hiding this comment.
Err but it's fine if it's a proper sentence? Unless we don't want sentence-type descriptions, then we should say that explicitly. But I don't think we want to do that.
There was a problem hiding this comment.
I don't think we're clear on this. I always took the rule to be that we don't want to terminate in a full-stop. I suppose if there's a previous full stop before the end, we might want another?
There was a problem hiding this comment.
Could I get explanation of that new rule? I'm unable to understand it.
There was a problem hiding this comment.
Long time ago this was considered for repoman... and, if I don't misremember, it was finally reverted because of people thinking differently on this :/
https://bugs.gentoo.org/438976
|
Is there a policy...ish... decision on this? Specifically something that renders it into "this should be enforced". If not I'd like to close this. |
|
@mgorny @ulm @arthurzam : is this desired gentoo stylistic standard? If not, I'm going to close this out for staleness reasons. |
|
Yes, it's a Gentoo style thing. I think we want this, as a style-level result in pkgcheck. |
|
So... You took I was nudging for someone to reassure t this, right? :-) |
Fucking cellphone, apologies for that garbled bit of english. At least this time it didn't autocorrect into french. Rephrasing since people got an email: can someone update this and pull it across the line? Again, I want to clean out stale stuff since it's hard to ask people to open PRs if we don't ever get to them due to the queue. |
|
Personally, I think we don't want a full stop at the end of
which is a complete sentence but without a full stop. The question is of course whether we consider an example from |
|
My stance for pkgcheck, specifically for stylistic enforcement, is the dev community defines it, not us. We find mistakes and déviations from standard, rather than define the standard and force it on people. I agree with what you advocate,,just to be clear. I just want actual ebuild devs to decide this; pluriel, to be clear. |
|
Quick grep of the tree:
Looks like the last category will preclude any simple automatic check. |
|
I dare say you can hardwire the few abbreviations that are used. Even literal |
|
On top of that, probably at least some of the "complete sentences" should be reported for repeating the package name anyway. |
|
I'll update it. |
Bug: https://bugs.gentoo.org/729968 Signed-off-by: Arthur Zamarin <arthurzam@gentoo.org> Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
(Rebased so far, not yet final.) |
Signed-off-by: Michał Górny <mgorny@gentoo.org>
|
https://codeberg.org/gentoo/gentoo/pulls/737 covers the packages that would be reported. |
| yield BadDescription("generic eclass defined description", pkg_desc=desc, pkg=pkg) | ||
| elif s in (pkg.package.lower(), pkg.key.lower()): | ||
| yield BadDescription("generic package description", pkg_desc=desc, pkg=pkg) | ||
| elif desc.endswith(tuple(".,:;")) and not desc.lower().endswith(("etc.", "inc.", "...")): |
There was a problem hiding this comment.
WFM, but if we include Inc. then we should also include Co.,Ltd. and e.V. (And I hope we won't play whack-a-mole ...)
There was a problem hiding this comment.
I get wanting to include other common US names, but I don't think we should cover all the countries over the world.
There was a problem hiding this comment.
Whatever. This is a heuristic anyway, and etc./co./inc./ltd./... will cover everything existing in the tree.
Signed-off-by: Michał Górny <mgorny@gentoo.org>
6 participants
Bug: https://bugs.gentoo.org/729968
results on gentoo tree