-
Notifications
You must be signed in to change notification settings - Fork 25
Conversation
…ror and rest added, README updated
… description from the screen
…rk with more information (#95)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry for not focusing much on the app / logic / flows, i'm new to this application so can not see the bigger picture yet.
Most comments are small and good practice proposals I have learned from the past.
electron/config_storage.js
Outdated
@@ -1,40 +1,48 @@ | |||
var electron = window.require('electron'); | |||
const Config = electron.remote.require('electron-config') | |||
var electron = require('electron'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason not to use const
here?
electron/config_storage.js
Outdated
* | ||
* @see https://github.com/sindresorhus/electron-config | ||
*/ | ||
export function setConfig(key, value) { | ||
let setConfig = function(key, value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could keep it simple with just
function setConfig(...
same for getter
electron/config_storage.js
Outdated
|
||
module.exports = global.configStorage = configStorage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no newline at end of file is bad for git diffs
return total | ||
}, {})) | ||
|
||
module.exports = global.log = logger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
white line at eof
@@ -2,6 +2,7 @@ const electron = require('electron'); | |||
const fs = require('fs'); | |||
const os = require('os'); | |||
const path = require('path'); | |||
const log = require('./debug_handler.js') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can require().getLogger('FILE_IDENTIFIER')
to not repeat the 'MAIN_PROCESS > GOLEM_HANDLER'
later
getLogger
is pseudo code, not sure if it exists ( yet ;) )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice idea! I'll implement it soon.
index.js
Outdated
@@ -7,7 +7,9 @@ var mkdirp = require('mkdirp'); | |||
|
|||
//require('electron-debug')({showDevTools: true, enabled: true}); | |||
|
|||
const createTray = require('./electron/tray_handler.js') | |||
//const createTray = require('./electron/tray_handler.js') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
commented out to stay? we used to only allow commented out code if it has an explanation comment.
src/components/tasks/TaskDetail.js
Outdated
@@ -206,14 +221,14 @@ export class TaskDetail extends React.Component { | |||
} | |||
|
|||
_convertPriceAsHR(price) { | |||
console.log("price", price); | |||
// console.log("price", price); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logs to stay? could become log.debug
or log.silly
src/reducers/frame/single.js
Outdated
@@ -1,6 +1,6 @@ | |||
import { dict } from './../../actions' | |||
|
|||
const {SET_FRAMES_WITH_SUBTASKS, SET_SUBTASKS_BORDER, SET_PREVIEW_LIST, SET_SUBTASKS_LIST, SET_SUBTASKS_VISIBILITY, SET_FRAME_ID, NEXT_FRAME, PREVIOUS_FRAME} = dict | |||
const {SET_FRAMES_WITH_SUBTASKS, SET_SUBTASKS_BORDER, SET_PREVIEW_LIST, SET_SUBTASKS_LIST, SET_SUBTASKS_VISIBILITY, SET_FRAME_ID, NEXT_FRAME, PREVIOUS_FRAME, CLEAR_TASK_PLAIN} = dict |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe break down long line?
src/sagas/frame/index.js
Outdated
@@ -161,6 +161,7 @@ export function fetchSubtaskList(session, payload) { | |||
} | |||
|
|||
export function* subtaskList(session, payload) { | |||
console.log("payload", payload); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
log.verbose
or more debuggy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug
*/ | ||
export function* statsFlow(session) { | ||
yield fork(fireBase, session) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eof
@mdtanrikulu Can we merge this, pleaaaase? |
Sure, let me check after merging dev into developer mode, then I'll merge it. |
… electronLayer, developer mode interaction is live from now on
To activate/deactivate developer mode;
Windows, Linux:
Ctrl + Shift + D
Mac:
cmd⌘ + Shift + D
To activate/deactivate debug mode;
Windows, Linux:
Ctrl + Shift + L
Mac:
cmd⌘ + Shift + L