Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
clean up subiquity internals a bit #167
Conversation
mwhudson
added some commits
Sep 27, 2016
| + result = { | ||
| + 'realname': email, | ||
| + 'username': email, | ||
| + 'passwod': '', |
| + result = { | ||
| + 'realname': email, | ||
| + 'username': data['username'], | ||
| + 'passwod': '', |
warsaw
Sep 27, 2016
•
Edited 1 time
-
warsaw
Sep 27, 2016
-
Sep 27, 2016
"password" perhaps?
Although now that I've read more of the old code below, I see the field used to be 'passwod' so you're not changing that. Why the deliberate mispelling? Perhaps a comment would be helpful to explain what's going on here?
mwhudson
Sep 28, 2016
Collaborator
Yeah, I presume this is an irrelevant typo that can be fixed but I haven't checked. All this code is nonsense anyway.
| - "Identity": None, | ||
| - "Login": None, | ||
| - } | ||
| + controllers = [ |
warsaw
Sep 27, 2016
I don't know the code, but I notice that this is changing a dict with None values to a list. If order doesn't matter, perhaps a set would work better?
| @@ -23,6 +23,13 @@ | ||
| class CephDiskController(BaseController): | ||
| + signals = [ |
warsaw
Sep 27, 2016
I don't know how the code is used, but would a dictionary be better here... and below?
mwhudson
Sep 28, 2016
Collaborator
Possibly, this reflects the API of urwid.connect_signal more closely, which is what all this signal stuff backs onto. I may attempt to delete all the signal stuff in favour of calling methods on objects fairly soon...
| - self.set_alarm_in(0.05, self.welcome) | ||
| - for k in self.controllers.keys(): | ||
| + self.set_alarm_in(0.05, self.next_screen) | ||
| + for k in self.common['controllers'].keys(): |
warsaw
Sep 27, 2016
Of course, if self.common['controllers'] is a dictionary, you don't need .keys() since that's the default iteration strategy for dictionaries.
warsaw
reviewed
Sep 27, 2016
I'm not really familiar with the code, but I added a few comments on things that stood out to me.
|
Thanks for the comments. I think the codebase is a bit over-designed at present, I'd love your opinion on general structure too! |
mwhudson
added some commits
Sep 28, 2016
mwhudson commentedSep 27, 2016
The main change is moving the signal->method mapping to the controller which makes sooo much more sense to me. More cleanup to come, but it's late and I'm off tomorrow so sending this along.