Skip to content
This repository has been archived by the owner on Jun 4, 2024. It is now read-only.

first pass at unifying communication #27

Merged
merged 39 commits into from
Jun 23, 2016
Merged

first pass at unifying communication #27

merged 39 commits into from
Jun 23, 2016

Conversation

chriddyp
Copy link
Member

@chriddyp chriddyp commented Jun 9, 2016

No description provided.

chriddyp and others added 22 commits June 9, 2016 15:01
@chriddyp
first pass at unifying communication
527cce8
@alexandresobolevski
📖 rename ipcMessageHandler and move IPC_TASKS into it
83465bc
@alexandresobolevski
🛀 remove unexacting dependencies
ce1bf61
@alexandresobolevski
🔧 define the channel name in the messageHandler
c07e9de
@alexandresobolevski
🔧 promisify log
8ca9b59
@alexandresobolevski
🔧 write all cases to handle server & ipc calls
dc15f08
@alexandresobolevski
🐄 simplify code by writing helper functions
73fbf18
@alexandresobolevski
🔧 rework the table preview, new helper fun
164dbf3
@alexandresobolevski
🔧 login log update is in the messageHandler
c6dbf63 
no callback needed here
@alexandresobolevski
🛀
7cedc5a
@alexandresobolevski
🔧 support server connect call without creds
80d7870
@alexandresobolevski
🔧 use callback in sendQuery as well
db25838
@alexandresobolevski
🔧 rewrite disconnect: promisify and add callback
eb629c1
@alexandresobolevski
🛀
294c977
@alexandresobolevski
🔧 rewrite to support new format of table preview data
fd3a13c
@alexandresobolevski
🔧 move out input support functions out of render
7bca6a9
@alexandresobolevski
📖 rename IPC_TASKS because used by remote server too
9586aee
@alexandresobolevski
🔧 expose end points in main
df93755
@alexandresobolevski
🐄 📖
697fd8a
@alexandresobolevski
📖 was bad idea to not use same sequelize syntax
e1e70ea
@alexandresobolevski
🔧 fix server calls that require credentials
c7cde03
@alexandresobolevski
🛀
891bb02
@alexandresobolevski
Copy link
Contributor

alexandresobolevski commented Jun 13, 2016 •
edited
Loading

establish credentials with the app, check connection with a POST to localhost:5000/connect

get table previews from testdb database using a POST to localhost:5000/tables

disconnect using a POST to localhost:5000/disconnect

ready @chriddyp 😄

@alexandresobolevski alexandresobolevski mentioned this pull request Jun 13, 2016
Merged
@alexandresobolevski
Copy link
Contributor

ipcMessageHandler.js, serverMessageHandler.js, ipc.js are now simplified into one file messageHandler.js

The latter receives commands from either the app through the IPC or from a remote server through a POST to localhost:5000.

Within messageHandler.js the callback function will be decided depending on the origin of the request (server vs app). The next steps are decided by either the TASK given by through the ipc channel or the /route at which the POST was received at for the app requests and remote server requests respectively.

sequelizeManager manages only what type of data is part of the response using the callback function given to it by messageHandler.js.

chriddyp
chriddyp reviewed Jun 21, 2016
View reviewed changes
messageHandler.js
Object.keys(config).forEach( key => {
setup[key] = config[key];
});
return setup;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this just be written as:

const setup = R.merge(options, config);

?

chriddyp
chriddyp reviewed Jun 21, 2016
View reviewed changes
main.development.js
@@ -43,7 +56,17 @@ app.on('ready', () => {
mainWindow.focus();
ipcMain.removeAllListeners(channel);
ipcMain.on(channel, ipcMessageHandler(sequelizeManager));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍

@chriddyp
Copy link
Member Author

looks great! this is coming out super clean 🛁 :)

just a couple things around:

  • some variable name changing
  • some un-promisifying synchornous code

💃 when those things are updated, or feel free to ping me for another review.

@alexandresobolevski alexandresobolevski self-assigned this Jun 21, 2016
@alexandresobolevski alexandresobolevski mentioned this pull request Jun 22, 2016
Closed
@alexandresobolevski
🛀 add editorconfig as per PR comment ->
e6362a5 
#27 (comment)
17122
@alexandresobolevski
🛀 clean comments as per PR comment ->
dde584e 
#27 (comment)
-67915912
@alexandresobolevski
🔧 use ramda merge to simplify PR comment ->
4aa05ec 
#27 (comment)
11426
@alexandresobolevski
📖 too many Handler functions
9e0b2d2
@alexandresobolevski
📖 use constants for dialect names everywhere
56f9d39 
#27 (comment)
22756
@alexandresobolevski
📖 no more ENGINE - the era of DIALECT is upon us
22404d7
@alexandresobolevski
📖 the era of merge is over - use update for clarity
6c51d70
@alexandresobolevski
🐄 define component.propTypes and warnings be gone!
ac47fea
@alexandresobolevski
🐨 add x scrolls to the errorMessage
8de3d4a
@alexandresobolevski
🐛 📖 fix warnings due to message being a key within a key
0ebc6a2 
renamed to `description`
@alexandresobolevski
🔧 un-promisify sequelizeManager.log and .disconnect
1e71b94 
https://github.com/plotly/electron-sql-connector/pull/27/files#r67920678
and
https://github.com/plotly/electron-sql-connector/pull/27/files#r67915451
@alexandresobolevski
🔧 use raiseError instead of just callback for better flow control
642a8b8
@alexandresobolevski
🛀
f7b949b
@alexandresobolevski alexandresobolevski mentioned this pull request Jun 23, 2016
Closed
@alexandresobolevski alexandresobolevski merged commit d693e1d into UX-V1 Jun 23, 2016
@alexandresobolevski alexandresobolevski deleted the unify-comm branch June 28, 2016 16:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees

@alexandresobolevski alexandresobolevski

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chriddyp @alexandresobolevski