timhot
commented
5 years ago thanks.
Should I tidy those things up before the merge or after?
Tim
EDIT - Decided to: 1) Merge 2) Create a new branch where I am tiding up code and adding automated testing back in (old tests no longer work due to major GUI changes).
From: Menno Finlay-Smits notifications@github.com Sent: Friday, 15 February 2019 5:52 PM To: TheCacophonyProject/cacophonometer Cc: Tim Hunt; Author Subject: Re: [TheCacophonyProject/cacophonometer] More gui work (#18)
@mjs approved this pull request.
There's a lot here! Looks good to me. Just some little things.
In app/src/main/java/nz/org/cacophony/cacophonometer/TestRecordFragment.javahttps://github.com/TheCacophonyProject/cacophonometer/pull/18#discussion_r257102241:
- } else if (messageType.equalsIgnoreCase("GETTING_READY_TO_RECORD")) {
- tvMessages.setText(messageToDisplay);
- } else if (messageType.equalsIgnoreCase("FAILED_RECORDINGS_NOT_UPLOADED")) {
- tvMessages.setText(messageToDisplay);
- } else if (messageType.equalsIgnoreCase("RECORD_AND_UPLOAD_FAILED")) {
- tvMessages.setText(messageToDisplay);
- } else if (messageType.equalsIgnoreCase("UPLOADING_FAILED_NOT_REGISTERED")) {
- tvMessages.setText(messageToDisplay);
- } else if (messageType.equalsIgnoreCase("RECORDING_STARTED")) {
- tvMessages.setText(messageToDisplay);
- } else if (messageType.equalsIgnoreCase("RECORDING_FINISHED")) {
- tvMessages.setText(messageToDisplay);
- //getView().findViewById(R.id.btnRecordNow).setEnabled(true);
- btnRecordNow.setEnabled(true);
- btnRecordNow.setVisibility(View.VISIBLE);
- }
if messageType is made uppercase first then you could use a switch statement here instead. That would probably be a little less verbose.
In app/src/main/java/nz/org/cacophony/cacophonometer/TestingFragment.javahttps://github.com/TheCacophonyProject/cacophonometer/pull/18#discussion_r257102441:
- }
- Util.setUseTestServer(getActivity().getApplicationContext(), isChecked);
+// swShortRecordings.setEnabled(isChecked); +// swUseVeryFrequentRecordings.setEnabled(isChecked);
- if (!isChecked){
- Prefs prefs = new Prefs(getActivity().getApplicationContext());
- prefs.setUseShortRecordings(false); +// swShortRecordings.setChecked(false); +// swShortRecordings.setEnabled(false);
- prefs.setUseVeryFrequentRecordings(false); +// swUseVeryFrequentRecordings.setChecked(false); +// swUseVeryFrequentRecordings.setEnabled(false);
Do these commented out sections have to stay around?
In app/src/main/java/nz/org/cacophony/cacophonometer/Util.javahttps://github.com/TheCacophonyProject/cacophonometer/pull/18#discussion_r257102530:
- return "";
- }
- String webTokenBody = Util.decoded(webToken);
- JSONObject jObject = new JSONObject(webTokenBody);
- return jObject.getString("deviceName");
- } +// static String getDeviceName(String webToken) throws Exception { +// if (webToken == null){ +// return ""; +// } +// +// String webTokenBody = Util.decoded(webToken); +// JSONObject jObject = new JSONObject(webTokenBody); +// return jObject.getString("deviceName"); +// }
delete?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/TheCacophonyProject/cacophonometer/pull/18#pullrequestreview-204077856, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ADtSVOQb_BhqY0ce_aSkTP8aNFgCEGmEks5vNj0GgaJpZM4a6vfo.
Mostly finished work on new GUI so want to merge back into Master Next - once back in Master, I'll add automatic testing back in and prune dead code/comments