On 6/10/24 16:55, Aaron Conole wrote: > Eelco Chaudron <[email protected]> writes: > >> On 7 Jun 2024, at 15:46, Mike Pattrick wrote: >> >>> On Fri, Jun 7, 2024 at 2:35 AM Eelco Chaudron <[email protected]> wrote: >>>> >>>> >>>> >>>> On 6 Jun 2024, at 3:07, Mike Pattrick wrote: >>>> >>>>> This patch extends the extra_keywords list from 324 to 747 keywords and >>>>> moves this list to a separate file. The methodology used to create this >>>>> list was running the spell checker on a large volume of historical >>>>> patches and selecting any words that appeared multiple times. >>>> >>>> Thanks Mike, >>>> >>>> I like the idea of having this in a separate file (I would add the >>>> .txt extension to it), however, just blindly taking the last x >>>> errors does not seem to be the right approach. >>>> >>>> Last time I took the words from the last 1000 commits that made >>>> sense. For example, things like countersfn, deviceiocontrol, >>>> etc. do not make sense to me to add. >>> >>> Why wouldn't we want something like deviceiocontrol in an exclusion >>> list? It's a common Windows function name, any commit that touches the >>> windows code has a high likelihood of including it. >> >> Well deviceiocontrol is maybe an outlier, but there is a lot of other >> stuff we should not add. And even though, it only checks comments and >> commit messages.
I agree with Eelco on that. Some parts of the code will inevitably be mentioned in commit messages or comments and we can't add all of them in the dictionary. Even 'deviceiocontrol' doesn't seem like a good word for the dictionary. It might be better to work on better recognition of code parts or quoted words and strings within comments and commit messages. Some added words are just abbreviated normal words, so it may be better to encourage people to use a full word instead by not adding to the dictionary. Some added words like 'lexograpically' seem like just legit typos. This is not how this word supposed to be spelled. >> >> To make it easier to review the words added, maybe split the patch in >> two patches. One moving words to checkpatch.words, or some other self >> explanatory name, and the other one introducing the additional words. > > I agree with this approach. It is a bit difficult to review when moving > things and introducing changes in the same patch. Better to split it to > the move and then the additions. +1 Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
