KML routes of trackables (#62) #70
Conversation
| @@ -124,3 +124,11 @@ def test_post_log(self, mock_request, mock_load_log_page): | |||
| "ctl00$ContentBody$LogBookPanel1$uxLogInfo": test_log_text, | |||
| } | |||
| mock_request.assert_called_with(self.t._log_page_url, method="POST", data=expected_post_data) | |||
|
|
|||
| def test_load_KML(self): | |||
There was a problem hiding this comment.
Please rename the test method according to the tested method. In this case test_get_KML().
| @@ -179,6 +199,7 @@ def load(self): | |||
| self.owner = root.find(id="ctl00_ContentBody_BugDetails_BugOwner").text | |||
| self.goal = root.find(id="TrackableGoal").text | |||
| self.description = root.find(id="TrackableDetails").text | |||
| self.kmlurl = root.find(id="ctl00_ContentBody_lnkGoogleKML").get("href") | |||
There was a problem hiding this comment.
Can you please make 2 more changes?
-
rename the
kmlurltokml_urlaccording to PEP8:Method Names and Instance Variables
Use the function naming rules: lowercase with words separated by underscores as necessary to improve readability.
-
Make the variable "non-public" (as defined in PEP8) – add one leading underscore – so the final name will be
_kml_url. Then you can delete the new@propertyand@kmlurl.settermethods and handle the potentially missing URL in theget_KML()method the same way like in the_load_log_page()method.The goal of this change is to hide this variable from
Trackablepublic API.
|
Thanks for your great work! Just a few more changes and we are ready to go. |
No description provided.