Merge ~jugmac00/lazr.sshserver:apply-inclusive-naming into lazr.sshserver:main
- Git
- lp:~jugmac00/lazr.sshserver
- apply-inclusive-naming
- Merge into main
Status: | Merged |
---|---|
Merged at revision: | e2f4e20624f1807cbee11454e3162a8699c8dccd |
Proposed branch: | ~jugmac00/lazr.sshserver:apply-inclusive-naming |
Merge into: | lazr.sshserver:main |
Diff against target: |
310 lines (+243/-7) 5 files modified
.pre-commit-config.yaml (+4/-0) .woke.yaml (+228/-0) NEWS.txt (+1/-0) src/lazr/sshserver/session.py (+8/-4) src/lazr/sshserver/tests/test_auth.py (+2/-3) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Colin Watson (community) | Approve | ||
Review via email: mp+411680@code.launchpad.net |
Commit message
Apply inclusive naming via the woke pre-commit hook
Description of the change
Jürgen Gmach (jugmac00) wrote (last edit ): | # |
Colin Watson (cjwatson) wrote : | # |
`_DummyTransport` is a Twisted name - it's in `twisted.
Colin Watson (cjwatson) : | # |
Jürgen Gmach (jugmac00) wrote : | # |
> `_DummyTransport` is a Twisted name - it's in `twisted.
> Given that that's outside our control, I think we should revert those changes
> here and add exemptions for them.
Thank you for the reference.
Afaik you can only add inline ignores or next line ignores, while it still looks odd, I tried to come up with a readable solution.
I also created a discussion at https:/
Can I have another review? Thank you.
Colin Watson (cjwatson) : | # |
Preview Diff
1 | diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml |
2 | index d4436ec..b6827cc 100644 |
3 | --- a/.pre-commit-config.yaml |
4 | +++ b/.pre-commit-config.yaml |
5 | @@ -26,3 +26,7 @@ repos: |
6 | rev: 5.10.1 |
7 | hooks: |
8 | - id: isort |
9 | +- repo: https://github.com/get-woke/woke |
10 | + rev: v0.17.0 |
11 | + hooks: |
12 | + - id: woke-from-source |
13 | diff --git a/.woke.yaml b/.woke.yaml |
14 | new file mode 100644 |
15 | index 0000000..71c13eb |
16 | --- /dev/null |
17 | +++ b/.woke.yaml |
18 | @@ -0,0 +1,228 @@ |
19 | + |
20 | +# Based on Canonical's Guidelines |
21 | +# https://docs.google.com/document/d/1mJUa1VJHOMWa723dmKmNjSKGW-nlBv9xktzGZZwacVo/edit#heading=h.5efudb237qdb |
22 | +rules: |
23 | + - name: whitelist |
24 | + terms: |
25 | + - whitelist |
26 | + - white-list |
27 | + - whitelisted |
28 | + - white-listed |
29 | + alternatives: |
30 | + - allowlist |
31 | + - inclusion list |
32 | + severity: warning |
33 | + note: "The underlying assumption of the whitelist/blacklist metaphor is that white = good and black = bad. Because colors in and of themselves have no predetermined meaning, any meaning we assign to them is cultural: for example, the color red in many Southeast Asian countries is lucky, and is often associated with events like marriages, whereas the color white carries the same connotations in many European countries. In the case of whitelist/blacklist, the terms originate in the publishing industry – one dominated by the USA and England, two countries which participated in slavery and which grapple with their racist legacies to this day." |
34 | + |
35 | + - name: blacklist |
36 | + terms: |
37 | + - blacklist |
38 | + - black-list |
39 | + - blacklisted |
40 | + - black-listed |
41 | + alternatives: |
42 | + - denylist |
43 | + - blocklist |
44 | + - exclusion list |
45 | + severity: warning |
46 | + note: "The underlying assumption of the whitelist/blacklist metaphor is that white = good and black = bad. Because colors in and of themselves have no predetermined meaning, any meaning we assign to them is cultural: for example, the color red in many Southeast Asian countries is lucky, and is often associated with events like marriages, whereas the color white carries the same connotations in many European countries. In the case of whitelist/blacklist, the terms originate in the publishing industry – one dominated by the USA and England, two countries which participated in slavery and which grapple with their racist legacies to this day." |
47 | + |
48 | + - name: master-slave |
49 | + terms: |
50 | + - master-slave |
51 | + - master/slave |
52 | + alternatives: |
53 | + - leader/follower |
54 | + - main/replica |
55 | + - primary/replica |
56 | + - primary/standby |
57 | + - primary/secondary |
58 | + |
59 | + - name: slave |
60 | + terms: |
61 | + - slave |
62 | + alternatives: |
63 | + - follower |
64 | + - replica |
65 | + - standby |
66 | + |
67 | + - name: grandfathered |
68 | + terms: |
69 | + - grandfathered |
70 | + alternatives: |
71 | + - legacy status |
72 | + - legacied |
73 | + - exempted |
74 | + - carried |
75 | + - brought forward |
76 | + - rolled over |
77 | + |
78 | + - name: man-hours |
79 | + terms: |
80 | + - man hours |
81 | + - man-hours |
82 | + alternatives: |
83 | + - person hours |
84 | + - engineer hours |
85 | + |
86 | + - name: sanity |
87 | + terms: |
88 | + - sanity |
89 | + alternatives: |
90 | + - confidence |
91 | + - quick check |
92 | + - coherence check |
93 | + |
94 | + - name: dummy |
95 | + terms: |
96 | + - dummy |
97 | + alternatives: |
98 | + - placeholder |
99 | + - sample |
100 | + |
101 | + - name: guys |
102 | + terms: |
103 | + - guys |
104 | + alternatives: |
105 | + - folks |
106 | + - people |
107 | + - you all |
108 | + - y'all |
109 | + - yinz |
110 | + |
111 | + - name: whitebox |
112 | + terms: |
113 | + - white-box |
114 | + - whitebox |
115 | + - white box |
116 | + alternatives: |
117 | + - open-box |
118 | + |
119 | + - name: blackbox |
120 | + terms: |
121 | + - black-box |
122 | + - blackbox |
123 | + - black box |
124 | + alternatives: |
125 | + - closed-box |
126 | + |
127 | + - name: blackhat |
128 | + terms: |
129 | + - blackhat |
130 | + - black-hat |
131 | + - black hat |
132 | + alternatives: |
133 | + - attacker |
134 | + - malicious actor |
135 | + |
136 | + - name: whitehat |
137 | + terms: |
138 | + - whitehat |
139 | + - white-hat |
140 | + - white hat |
141 | + alternatives: |
142 | + - researcher |
143 | + - security specialist |
144 | + |
145 | + - name: illegal characters |
146 | + terms: |
147 | + - illegal characters |
148 | + alternatives: |
149 | + - invalid characters |
150 | + - unsupported characters |
151 | + - special characters |
152 | + |
153 | + - name: native feature |
154 | + terms: |
155 | + - native feature |
156 | + alternatives: |
157 | + - core feature |
158 | + - built-in feature |
159 | + |
160 | + - name: native feature |
161 | + terms: |
162 | + - native feature |
163 | + alternatives: |
164 | + - core feature |
165 | + - built-in feature |
166 | + |
167 | + - name: chairman/foreman |
168 | + terms: |
169 | + - chairman |
170 | + - foreman |
171 | + alternatives: |
172 | + - chair |
173 | + - foreperson |
174 | + |
175 | + - name: man in the middle |
176 | + terms: |
177 | + - man in the middle |
178 | + - man-in-the-middle |
179 | + alternatives: |
180 | + - machine-in-the-middle |
181 | + - person-in-the-middle |
182 | + - system-in-the-middle |
183 | + - intermediary attack |
184 | + |
185 | + - name: middleman |
186 | + terms: |
187 | + - middleman |
188 | + alternatives: |
189 | + - middleperson |
190 | + - intermediary |
191 | + |
192 | + - name: manned |
193 | + terms: |
194 | + - manned |
195 | + alternatives: |
196 | + - crewed |
197 | + - staffed |
198 | + - monitored |
199 | + - human operated |
200 | + |
201 | + - name: mom test / girlfriend test |
202 | + terms: |
203 | + - mom test |
204 | + - girlfriend test |
205 | + alternatives: |
206 | + - user test |
207 | + - user friendly |
208 | + |
209 | + - name: crazy |
210 | + terms: |
211 | + - crazy |
212 | + alternatives: |
213 | + - baffling |
214 | + - unexplained |
215 | + - errant |
216 | + |
217 | + - name: cripples |
218 | + terms: |
219 | + - cripples |
220 | + alternatives: |
221 | + - slows down |
222 | + - hinders |
223 | + - obstructs |
224 | + |
225 | + - name: crippling |
226 | + terms: |
227 | + - crippling |
228 | + alternatives: |
229 | + - attenuating |
230 | + - incapacitating |
231 | + |
232 | + - name: stonith/stomith |
233 | + terms: |
234 | + - stonith |
235 | + - stomith |
236 | + alternatives: |
237 | + - fence failed nodes |
238 | + - machines |
239 | + |
240 | + - name: demilitarized zone |
241 | + terms: |
242 | + - demilitarized zone |
243 | + - dmz |
244 | + alternatives: |
245 | + - perimeter network |
246 | + - passthrough network |
247 | diff --git a/NEWS.txt b/NEWS.txt |
248 | index 43c2138..821a9cc 100644 |
249 | --- a/NEWS.txt |
250 | +++ b/NEWS.txt |
251 | @@ -9,6 +9,7 @@ NEWS for lazr.sshserver |
252 | - Add basic pre-commit configuration. |
253 | - Apply black code formatter. |
254 | - Add isort pre-commit hook. |
255 | +- Apply inclusive naming via the woke pre-commit hook. |
256 | |
257 | 0.1.12 (2021-09-13) |
258 | =================== |
259 | diff --git a/src/lazr/sshserver/session.py b/src/lazr/sshserver/session.py |
260 | index 29bde10..cdeefc3 100644 |
261 | --- a/src/lazr/sshserver/session.py |
262 | +++ b/src/lazr/sshserver/session.py |
263 | @@ -57,8 +57,10 @@ class PatchedSSHSession(session.SSHSession, object): |
264 | # necessary. See http://twistedmatrix.com/trac/ticket/2754. |
265 | transport = getattr(self.client, "transport", None) |
266 | if transport is not None: |
267 | - # For SFTP connections, 'transport' is actually a _DummyTransport |
268 | - # instance. Neither _DummyTransport nor the protocol it wraps |
269 | + # name cannot be changed as it refers to a name in |
270 | + # `twisted.conch.ssh.session`` |
271 | + # For SFTP connections, 'transport' is actually a _DummyTransport # wokeignore:rule=dummy # noqa: E501 |
272 | + # instance. Neither _DummyTransport nor the protocol it wraps # wokeignore:rule=dummy # noqa: E501 |
273 | # (filetransfer.FileTransferServer) support pausing. |
274 | pauseProducing = getattr(transport, "pauseProducing", None) |
275 | if pauseProducing is not None: |
276 | @@ -76,8 +78,10 @@ class PatchedSSHSession(session.SSHSession, object): |
277 | # necessary. See http://twistedmatrix.com/trac/ticket/2754. |
278 | transport = getattr(self.client, "transport", None) |
279 | if transport is not None: |
280 | - # For SFTP connections, 'transport' is actually a _DummyTransport |
281 | - # instance. Neither _DummyTransport nor the protocol it wraps |
282 | + # name cannot be changed as it refers to a name in |
283 | + # `twisted.conch.ssh.session` |
284 | + # For SFTP connections, 'transport' is actually a _DummyTransport # wokeignore:rule=dummy # noqa: E501 |
285 | + # instance. Neither _DummyTransport nor the protocol it wraps # wokeignore:rule=dummy # noqa: E501 |
286 | # (filetransfer.FileTransferServer) support pausing. |
287 | resumeProducing = getattr(transport, "resumeProducing", None) |
288 | if resumeProducing is not None: |
289 | diff --git a/src/lazr/sshserver/tests/test_auth.py b/src/lazr/sshserver/tests/test_auth.py |
290 | index 7fb7f21..58c7dae 100644 |
291 | --- a/src/lazr/sshserver/tests/test_auth.py |
292 | +++ b/src/lazr/sshserver/tests/test_auth.py |
293 | @@ -91,8 +91,8 @@ class MockSSHTransport(SSHServerTransport): |
294 | |
295 | def __init__(self, portal): |
296 | # In Twisted 8.0.1, Conch's transport starts referring to |
297 | - # currentEncryptions where it didn't before. Provide a dummy value for |
298 | - # it. |
299 | + # currentEncryptions where it didn't before. Provide a placeholder |
300 | + # value for it. |
301 | self.currentEncryptions = SSHCiphers("none", "none", "none", "none") |
302 | self.packets = [] |
303 | self.factory = self.Factory() |
304 | @@ -173,7 +173,6 @@ class TestUserAuthServer(UserAuthServerMixin, testtools.TestCase): |
305 | def test_requestRaisesConchError(self): |
306 | # ssh_USERAUTH_REQUEST should raise a ConchError if tryAuth returns |
307 | # None. Added to catch a bug noticed by pyflakes. |
308 | - # Whitebox test. |
309 | def mock_try_auth(kind, user, data): |
310 | return None |
311 |
@Colin - the replaced `_DummyTransport` did not look like a name, but more like an actual symbol. Maybe this refers to something in the past? Could not find that name, neither in this repo nor in LP from where it was split it seems.