coolaj86/greenlock.js-ARCHIVED
3
3
Fork 0
Code Issues 6 Pull Requests 1 Releases Activity

redundant Promise.then usage #14

New Issue
Closed
opened 2018-07-27 23:03:36 +00:00 by Ghost · 7 comments
Ghost commented 2018-07-27 23:03:36 +00:00
Copy Link

https://git.coolaj86.com/coolaj86/greenlock.js/src/branch/master/lib/core.js#L439

Doing

promise.then(function (results) {
  return results;
})

Is redundant, without that clause the promise will already resolve to results.

https://git.coolaj86.com/coolaj86/greenlock.js/src/branch/master/lib/core.js#L439 Doing ``` promise.then(function (results) { return results; }) ``` Is redundant, without that clause the promise will already resolve to `results`.
coolaj86 commented 2018-08-18 10:15:53 +00:00
Owner
Copy Link

Correct, this is safe to remove.

It's probably leftover from some debugging or a transformation of data, or a control flow branch that was removed.

I'll leave this open to remind me to take care of that.

Correct, this is safe to remove. It's probably leftover from some debugging or a transformation of data, or a control flow branch that was removed. I'll leave this open to remind me to take care of that.
Ghost commented 2018-10-26 08:27:05 +00:00
Author
Copy Link

AJ - do you have any objections to adding jest in for unit testing? I know this is a trivial change but that makes it useful for proving test automation.

Your hand rolled test code keeps things simple but I appreciate the extra info that using a framework provides.

AJ - do you have any objections to adding jest in for unit testing? I know this is a trivial change but that makes it useful for proving test automation. Your hand rolled test code keeps things simple but I appreciate the extra info that using a framework provides.
Ghost commented 2018-10-26 08:29:45 +00:00
Author
Copy Link

(btw - what the target minimum node level? I am working with 8 but tend to babel compile from 7+.)

Happy to create a pull request for this one, with or without jest

(btw - what the target minimum node level? I am working with 8 but tend to babel compile from 7+.) Happy to create a pull request for this one, with or without jest
coolaj86 commented 2018-10-26 22:22:58 +00:00
Owner
Copy Link

No objections.

No objections.
coolaj86 commented 2018-10-26 22:39:56 +00:00
Owner
Copy Link

Thank you, btw.

Thank you, btw.
Ghost commented 2018-11-05 11:21:02 +00:00
Author
Copy Link

Sorry about delay - I got stuck trying to wire a test around it. Not sure how to best test? Do you generally point it at the sandbox on an internet IP for testing?

Sorry about delay - I got stuck trying to wire a test around it. Not sure how to best test? Do you generally point it at the sandbox on an internet IP for testing?
coolaj86 commented 2018-11-06 08:16:51 +00:00
Owner
Copy Link

@chanoch Yes. I create a temporary digital ocean vps and point four test domains at it and run through a few different scenarios.

Then I deploy it to git.coolaj86.com and a possibly a few other sites to see if it works in my existing setups as well.

@chanoch Yes. I create a temporary digital ocean vps and point four test domains at it and run through a few different scenarios. Then I deploy it to git.coolaj86.com and a possibly a few other sites to see if it works in my existing setups as well.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
jshaver coolaj86
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: coolaj86/greenlock.js-ARCHIVED#14
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?