Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Update Dependencies and make this work #511

Merged
merged 20 commits into from Nov 25, 2017
Merged

[WIP] Update Dependencies and make this work #511

merged 20 commits into from Nov 25, 2017

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Nov 22, 2017

@hacdias
Copy link
Member Author

hacdias commented Nov 22, 2017

GREEN! @diasdavid

Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

This is HAWT 🔥! Thank you @hacdias :)

I noticed this codebase has no tests at all, want to add the first one?

"react-dom": "^15.1.0",
"react-widgets": "^3.2.0",
"react-dom": "^16.1.1",
"react-widgets": "^4.1.1",
"winston": "^2.1.0"
},
"devDependencies": {
"babel-core": "^6.3.21",
Copy link
Member

Choose a reason for hiding this comment

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

While you are at it, can you add aegir as a devDep and use its aegir lint to make sure the linting rules are the same as the remaining modules?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should I add it to webui too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done done 😄

@@ -4,50 +4,52 @@
"description": "IPFS Native Application",
Copy link
Member

Choose a reason for hiding this comment

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

Let's take off the 1.0.0-alpha.1, replace by 0.1.0 (that is a old pratice)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

src/init.js Outdated
ipfs.id()
.then((peer) => {
console.log(peer)
Copy link
Member

Choose a reason for hiding this comment

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

Unnecessary console.log

@hacdias
Copy link
Member Author

hacdias commented Nov 24, 2017

@diasdavid changed made!

package.json Outdated
@@ -53,8 +56,8 @@
},
"scripts": {
"start": "electron src/index.js",
"test": "aegir lint",
"lint": "aegir lint",
"test": "eslint .",
Copy link
Member

Choose a reason for hiding this comment

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

We really need some tests here :)

@daviddias
Copy link
Member

@hacdias is drag and drop working?

@hacdias
Copy link
Member Author

hacdias commented Nov 24, 2017

I'm not sure. It doesn't give any error but it doesn't seem to be uploading either. I'll check that when I arrive home. On train now 🚃

@hacdias
Copy link
Member Author

hacdias commented Nov 24, 2017

@diasdavid yup, it's working. It doesn't send any message telling so tho.

@hacdias
Copy link
Member Author

hacdias commented Nov 24, 2017

I see we're using notifier to send notifications. Windows 10 API has changed in the last update and I think that's the reason why I'm not receiving any notification. Source: mikaelbr/node-notifier#208.

Well, look at this: https://www.microsoft.com/developerblog/2016/10/30/showing-native-windows-notifications-from-electron-using-nodert/

I'll check that better and see what I can do!

Edit 2:

@daviddias daviddias merged commit 4da9a71 into ipfs:master Nov 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants