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

Adds dockerized development environment #123

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

alaxalves
Copy link

@alaxalves alaxalves commented Apr 1, 2018

This PR proposes an entire wrapped development environment, making easier to collaborate.

@alaxalves
Copy link
Author

Any doubts or suggestions I'm here to help.

@ifedapoolarewaju
Copy link
Collaborator

@alaxalves wow thank you for the PR, I'll review and let you know

Copy link
Collaborator

@ifedapoolarewaju ifedapoolarewaju left a comment

Choose a reason for hiding this comment

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

Hi @alaxalves, sorry the review came a bit lit, but I added a few notes. Please let me know what you think.

README.md Outdated

4. Download Nut by running `curl -L https://github.com/matthieudelaro/nut/raw/manualbuild/release/linux/nut -o nut && chmod a+x nut`

4. Run `sudo mv nut /usr/local/bin/nut` to move the nut executable to you local binaries *optional step*
Copy link
Collaborator

Choose a reason for hiding this comment

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

typo here "you" -> "your" 🙂

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, haven't seen it. :D

nut.yml Outdated
actions:
- npm install
- npm run build
- npm run gui
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not just npm start, Which runs both gulp build && electron .?

Copy link
Author

Choose a reason for hiding this comment

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

npm start would fit here as well. Do you want me to change??

package.json Outdated
@@ -4,6 +4,8 @@
"description": "Desktop application for Instagram DMs",
"main": "main/main.js",
"scripts": {
"build": "gulp build",
"gui": "electron .",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure we need these scripts, especially if you agree with my suggestion above

Copy link
Author

@alaxalves alaxalves Apr 14, 2018

Choose a reason for hiding this comment

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

I needed to split those commands because I added the option to nut reload in the nut.yml file. This option allows you to build your code again and see the changes you have made in the software without having to drop the docker containers. Otherwise you'd have to re-run the docker container everytime you made a change and wanted it to make effect.

@pepeaste
Copy link

me sale "Cookie sessionid you are searching found was either not found or not valid!"
que hago

alaxalves added a commit to alaxalves/igdm that referenced this pull request May 1, 2018
run:
actions:
- npm install
- npm start
Copy link
Author

@alaxalves alaxalves May 1, 2018

Choose a reason for hiding this comment

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

@ifedapoolarewaju Hey I've changed the code according to the request you've made!

@Arwwaaa
Copy link

Arwwaaa commented Jun 2, 2018

I can't find the chats help me

@ifedapoolarewaju ifedapoolarewaju force-pushed the master branch 4 times, most recently from c614180 to f09f454 Compare November 9, 2019 13:20
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.

4 participants