Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#9014 defect closed invalid (invalid)

txchecker is incorrectly checking variable names in keydata.py

Reported by: the0id Owned by:
Priority: normal Milestone:
Component: release management Keywords:
Cc: Branch:
Author:

Description

privateED25519_openssh and publicED25519_openssh follow the naming convention for keys in keydata.py, but they fail the txchecker test for naming conventions.

Although privateED25519openssh and publicED25519openssh pass.

A fix for this could be changing the regular expression to:

(([A-Z_][A-Z0-9_]*)|(__.*__)|([a-z_][a-zA-Z0-9_]+)|([A-Z][a-zA-Z0-9]+))$

Change History (6)

comment:1 Changed 3 years ago by Jean-Paul Calderone

Why does keydata.py get its own naming convention?

comment:2 in reply to:  1 Changed 3 years ago by the0id

Replying to Jean-Paul Calderone:

Why does keydata.py get its own naming convention?

I have a feeling that it's not just for keydata.py, but my guess is they wanted to have a standard convention for readability.

comment:3 Changed 3 years ago by Jean-Paul Calderone

Please link to PRs when submitting a ticket for review.

comment:4 in reply to:  3 Changed 3 years ago by the0id

Replying to Jean-Paul Calderone:

Please link to PRs when submitting a ticket for review.

I don't have a PR to link here. I couldn't find where the txchecker rules are written, and have a feeling it's not something I have access to. So I thought I'd offer a suggestion here and have someone who can make the change do it. :)

comment:5 Changed 3 years ago by Glyph

Keywords: review removed
Resolution: invalid
Status: newclosed

The tool resides here: https://github.com/twisted/twistedchecker

Please re-file this ticket there, and submit any PRs against twistedchecker, except of course the one to upgrade its version :).

Thanks for noticing this issue though, and I do hope you contribute an update :).

(This is probably a case of the underscore-to-separate-prefix convention, which is documented in our coding standard, but in this case it's a suffix... so a violation of the letter, but perhaps not the spirit...)

comment:6 Changed 3 years ago by the0id

I have the PR here https://github.com/twisted/twistedchecker/pull/125.

The build is failing now, but it looks like what's causing the failure is fixed in another PR.

Note: See TracTickets for help on using tickets.