Reduce mode #221

tapaswenipathak · 2018-06-18T10:59:07Z

Branch for #212 as git rebase master is....

Updates in the linked PR.

cc @mapbox/platform-engine-room.

For some reason jest is not passing, I deleted the repo, cloned it and then again run npn run update-jest-snapshots, doesn't work. Tried with deleted package-lock.json and rm -rf node_modules/ and then again npm ci, this has worked before.

arunasank · 2018-06-18T12:33:35Z

For some reason jest is not passing, I deleted the repo, cloned it and then again run npn run update-jest-snapshots, doesn't work. Tried with deleted package-lock.json and rm -rf node_modules/ and then again npm ci, this has worked before.

👋 @tapasweni-pathak, it seems like some of the updates to the jest dependencies are malfunctioning. To fix:

Reset your version of package-lock.json to what's on master (So, revert the changes to package-lock.json alone on 9f219d9)
Run npm run update-jest-snapshots

I have tested that this works ^, and is the root cause. Let me know if it works for you! 🙂

tapaswenipathak · 2018-06-18T12:41:54Z

👋@arunasank I have to delete package-lock.json and re run npm ci once as I added a package, can't keep the master one. Plus as I wrote I delete the directory, reclone and install and run update-jest-snapshots it still doesn't work.

arunasank · 2018-06-18T12:52:34Z

👋@arunasank I have to delete package-lock.json and re run npm ci once as I added a package, can't keep the master one. Plus as I wrote I delete the directory, reclone and install and run update-jest-snapshots it still doesn't work.

The reason the jest portion is not working is because when you're deleting the package-lock.json and running npm install, the dependencies for jest are getting updated (along with the dependencies for the new packages you're interested in installing), and these updated jest dependencies seem to be malfunctioning on your branch and others. Since you need to delete package-lock.json for using your new packages, maybe you can:

Use the package-lock.json from master to update the jest snapshots
Then delete package-lock.json and install your new modules.

While this will fix the issue of updating the snapshots, tests will still fail on Travis, because of the updated package-lock.json, and the involvement of jest in some pre-tests. Going to check if there's a way to partially update the package-lock.json file.

tapaswenipathak · 2018-06-18T13:08:58Z

@arunasank We can voice on it, this is weird,

rm -rf ecs-watchbot/.
git clone git@github.com:mapbox/ecs-watchbot.git.
cd ecs-watchbot
git fetch origin
rm package-lock.json
npm install && npm link (was using npm ci)
npm run update-jest-snapshots

This is when I need to update package-lock.json. I used the updated package-lock.json and it passed. I plan to ticket in jest, just curious what exactly is happening. I have had this with jest before as well. I agree on dependencies updating, thanks for finding the time to write the notes.

arunasank · 2018-06-18T13:15:58Z

This is when I need to update package-lock.json. I used the updated package-lock.json and it passed.

👍 Glad it's fixed!

jakepruitt · 2018-06-18T20:27:42Z

.eslintrc

@@ -6,7 +6,7 @@
    "jest": true
  },
  "parserOptions": {
-    "ecmaVersion": "2017"
+    "ecmaVersion": 8


Why the switch to ecmaVersion 8 here?

I think ecmaVersion released in 2017 is 8th edition, so can be written as 2017 or 8.

jakepruitt · 2018-06-18T21:12:48Z

lib/template.js

+    notificationTopic: cf.ref(prefixed('NotificationTopic'))
+  };
+
+  const reduce = (prefixed, Resources, options, ref) => {


What is your goal with making this section a function, rather than putting this code inside the if statement on line 237?

Can change it to be inside the if statement on line 237, added it to be consistent with the other methods defined in the template like mount e.t.c.

jakepruitt · 2018-06-26T18:59:57Z

bin/watchbot-log.js

+/**
+ * watchbot-log "something that you want logged"
+ *   - or -
+ * echo "somehing that you want logged" | watchbot-log


We've removed watchbot-log from watchbot 4. Could you delete this file?

jakepruitt · 2018-06-26T19:02:22Z

lib/template.js

@@ -145,6 +155,39 @@ module.exports = (options = {}) => {
    );
  };

+	const reduce = (prefixed, Resources, options, ref) => {


I still think the fact that this function is only used once and modifies Resources, options and ref, we shouldn't have the code live in a function, but instead have it live inside of the if statement at line 227. If we do want to keep it as a function, we should have that function live right next to the if statement at line 227 so we can easily reference the function code.

CannotLoadTemplateError: Failed to parse CloudFormation template. ReferenceError: ref is not defined

jakepruitt · 2018-07-10T20:51:39Z

This looks good!

tapaswenipathak force-pushed the watchbot4 branch 6 times, most recently from bb7f2aa to 9f219d9 Compare June 18, 2018 12:31

tapaswenipathak force-pushed the watchbot4 branch from 9f219d9 to b2f0938 Compare June 18, 2018 12:34

tapaswenipathak force-pushed the watchbot4 branch from b2f0938 to 759b349 Compare June 18, 2018 13:03

jakepruitt reviewed Jun 18, 2018

View reviewed changes

jakepruitt mentioned this pull request Jun 18, 2018

Adding Reduce mode to ♻that container #212

Closed

1 task

jakepruitt reviewed Jun 18, 2018

View reviewed changes

tapaswenipathak force-pushed the watchbot4 branch 2 times, most recently from ecd4637 to 5f3f0f4 Compare June 22, 2018 17:43

jakepruitt suggested changes Jun 26, 2018

View reviewed changes

tapaswenipathak force-pushed the watchbot4 branch 2 times, most recently from 74644d7 to 36692a5 Compare June 27, 2018 06:18

jakepruitt force-pushed the watchbot4 branch 3 times, most recently from d80b833 to 7dd0e45 Compare July 6, 2018 14:57

tapaswenipathak and others added 6 commits July 10, 2018 12:13

ecmaVersion as number

9322c49

update jest

7f8d31a

fix error in template

db507bb

CannotLoadTemplateError: Failed to parse CloudFormation template. ReferenceError: ref is not defined

ecmascript back to original

dfa49a2

Add watchbot-progress binary

34d8d3f

Add it to index too

7ccb4ed

jakepruitt added 3 commits July 10, 2018 12:16

Install watchbot/progress

7a43a7b

/lib/template.js

7e64060

ProgressTable in env

89f239a

jakepruitt force-pushed the watchbot4 branch from ab95792 to 89f239a Compare July 10, 2018 19:20

jakepruitt added 5 commits July 10, 2018 12:20

4.7.2-0

46eba40

4.7.2-1

f9cf361

4.7.2-2

44bfd6b

bump

174fb98

Update snapshot

92d261f

jakepruitt approved these changes Jul 10, 2018

View reviewed changes

jakepruitt merged commit 49e87be into master Jul 10, 2018

jakepruitt deleted the watchbot4 branch July 10, 2018 20:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reduce mode #221

Reduce mode #221

tapaswenipathak commented Jun 18, 2018 •

edited

Loading

arunasank commented Jun 18, 2018 •

edited

Loading

tapaswenipathak commented Jun 18, 2018

arunasank commented Jun 18, 2018 •

edited

Loading

tapaswenipathak commented Jun 18, 2018 •

edited

Loading

arunasank commented Jun 18, 2018

jakepruitt Jun 18, 2018

tapaswenipathak Jun 19, 2018

jakepruitt Jun 18, 2018

tapaswenipathak Jun 19, 2018

jakepruitt Jun 26, 2018

jakepruitt Jun 26, 2018

jakepruitt commented Jul 10, 2018

Reduce mode #221

Reduce mode #221

Conversation

tapaswenipathak commented Jun 18, 2018 • edited Loading

arunasank commented Jun 18, 2018 • edited Loading

tapaswenipathak commented Jun 18, 2018

arunasank commented Jun 18, 2018 • edited Loading

tapaswenipathak commented Jun 18, 2018 • edited Loading

arunasank commented Jun 18, 2018

jakepruitt Jun 18, 2018

Choose a reason for hiding this comment

tapaswenipathak Jun 19, 2018

Choose a reason for hiding this comment

jakepruitt Jun 18, 2018

Choose a reason for hiding this comment

tapaswenipathak Jun 19, 2018

Choose a reason for hiding this comment

jakepruitt Jun 26, 2018

Choose a reason for hiding this comment

jakepruitt Jun 26, 2018

Choose a reason for hiding this comment

jakepruitt commented Jul 10, 2018

tapaswenipathak commented Jun 18, 2018 •

edited

Loading

arunasank commented Jun 18, 2018 •

edited

Loading

arunasank commented Jun 18, 2018 •

edited

Loading

tapaswenipathak commented Jun 18, 2018 •

edited

Loading