Skip to content
This repository has been archived by the owner on Jan 7, 2023. It is now read-only.

Feedback from Mumbai workshop #5

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

Conversation

indcoder
Copy link

No description provided.

1. Errata in app.js file path
2. Adding verification step for succeesful completion of commands
1. Errata in app.js file path
2. Adding verification step for succeesful completion of commands
Copy link
Member

@mikesir87 mikesir87 left a comment

Choose a reason for hiding this comment

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

Couple of comments and items of feedback.

docs_en/tutorial/our-application/index.md Outdated Show resolved Hide resolved
docs_en/tutorial/our-application/index.md Show resolved Hide resolved
docs_en/tutorial/sharing-our-app/index.md Outdated Show resolved Hide resolved
@@ -9,7 +9,7 @@ Pretty simple, right? Let's make the change.

## Updating our Source Code

1. In the `~/app/src/static/js/app.js` file, update line 56 to use the new empty text. ([Editing files in PWD tips here](/pwd-tips#editing-files))
1. In the `~/app/src/static/app.js` file, update line 56 to use the new empty text. ([Editing files in PWD tips here](/pwd-tips#editing-files))
Copy link
Member

Choose a reason for hiding this comment

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

The source is in app/src/static/js, but is a fairly recent change.

Copy link
Author

Choose a reason for hiding this comment

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

Do you know when the change was made? Bcos the folks faced this on Saturday?

Copy link
Member

Choose a reason for hiding this comment

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

Odd. It was made Friday my time, so let me run through it real quick and see if it's still an issue 👍

Copy link
Member

Choose a reason for hiding this comment

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

Mk. This is what it looks like now. It's interesting that you have updated instructions, but not the updated zip, as those happened at the same time. Not sure what might be causing that.

Screen Shot 2019-10-21 at 2 59 13 PM

@@ -67,7 +73,7 @@ So, let's do it!

When you're done watching the logs, exit out by hitting `Ctrl`+`C`.

1. Now, let's make a change to the app. In the `src/static/js/app.js` file, let's change the "Add Item" button to simply say
1. Now, let's make a change to the app. In the `src/static/app.js` file, let's change the "Add Item" button to simply say
Copy link
Member

Choose a reason for hiding this comment

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

The source is in src/static/js/app.js here too.

Copy link
Author

Choose a reason for hiding this comment

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

image

Here's where app.js was when it came across in the zip.

docs_en/tutorial/multi-container-apps/index.md Outdated Show resolved Hide resolved
docs_en/tutorial/multi-container-apps/index.md Outdated Show resolved Hide resolved
docs_en/tutorial/sharing-our-app/index.md Outdated Show resolved Hide resolved
indcoder and others added 5 commits October 22, 2019 00:20
Co-Authored-By: Michael Irwin <mikesir87@gmail.com>
Co-Authored-By: Michael Irwin <mikesir87@gmail.com>
Co-Authored-By: Michael Irwin <mikesir87@gmail.com>
Co-Authored-By: Michael Irwin <mikesir87@gmail.com>
Co-Authored-By: Michael Irwin <mikesir87@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants