Fix cached promised values not resolving properly in some platforms. #874 #878 #887 #893
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
jung-kim
commented
Mar 30, 2017
•
jung-kim
commented
- add readme for Ungit stopped working on Chrome 57 #887
- fix rare edge case where remote ref node is gone on reset operation
- rescursively resolve all promises before caching them. Fix cache value being promise in ubuntu #878 Doesn't work with Chrome on MacOS #874
- Retain commit messages when commit fails fix Commit message lost when git returns error code #882
campersau
reviewed
Mar 30, 2017
| if (prom.then) { | ||
| return prom.then(getHardValue); | ||
| } else { | ||
| return Bluebird.resolve(prom); |
There was a problem hiding this comment.
Strange... normally .resolve should already do this:
If value is a thenable (Promise-like object, like those returned by jQuery's $.ajax), returns a trusted Promise that assimilates the state of the thenable.
There was a problem hiding this comment.
yeah this is a terrifying find and i need more confirmation but behavior was replicate-able in ubuntu.
dregad
reviewed
Mar 30, 2017
README.md
Outdated
| @@ -99,6 +99,7 @@ Known issues | |||
| * If you're running MacOSX Mavericks and Ungit crashes after a few seconds; try updating npm and node. See [#259](https://github.com/FredrikNoren/ungit/issues/259) and [#249](https://github.com/FredrikNoren/ungit/issues/249) for details. | |||
| * Ubuntu users may have trouble installing because the node executable is named differently on Ubuntu, see [#401](https://github.com/FredrikNoren/ungit/issues/401) for details. | |||
| * Debian Wheezy's supported git and nodejs packages are too old, therefore download newest [git](https://github.com/git/git/releases) and [nodejs](https://nodejs.org/download/) tarballs and [build from source](http://www.control-escape.com/linux/lx-swinstall-tar.html). | |||
| * Adblocker may block Ungit! Some ad blockers, such as uBlock, doesn't like localhost api calls and assumes that it is a cross domain attack. Please whitelist `localhost:{ungit port number}`. [#887](https://github.com/FredrikNoren/ungit/issues/887) | |||
There was a problem hiding this comment.
Suggested changes:
- rewording:
Some ad blockers, such as [Adblock plus](https://adblockplus.org) and [uBlock](https://www.ublock.org/), don't like localhost api calls and assume that [...] - add reference to
[#892](https://github.com/FredrikNoren/ungit/issues/892)
Also, note that the host name it is not necessarily localhost (in my case, running ungit on Cloud9 workspace, I have a workspace-name.c9users.io hostname), so your hint could be misleading.
dregad
reviewed
Mar 31, 2017
README.md
Outdated
| @@ -99,6 +99,7 @@ Known issues | |||
| * If you're running MacOSX Mavericks and Ungit crashes after a few seconds; try updating npm and node. See [#259](https://github.com/FredrikNoren/ungit/issues/259) and [#249](https://github.com/FredrikNoren/ungit/issues/249) for details. | |||
| * Ubuntu users may have trouble installing because the node executable is named differently on Ubuntu, see [#401](https://github.com/FredrikNoren/ungit/issues/401) for details. | |||
| * Debian Wheezy's supported git and nodejs packages are too old, therefore download newest [git](https://github.com/git/git/releases) and [nodejs](https://nodejs.org/download/) tarballs and [build from source](http://www.control-escape.com/linux/lx-swinstall-tar.html). | |||
| * Adblocker may block Ungit! Some ad blockers, such as [Adblock plus](https://adblockplus.org) and [uBlock](https://www.ublock.org/), doesn't like localhost api calls and assumes that it is a cross domain attack. Please whitelist `{localhost|127.0.0.1|$UngitURL}:{ungit port number}`. [#887](https://github.com/FredrikNoren/ungit/issues/887) [#892](https://github.com/FredrikNoren/ungit/issues/892) | |||
There was a problem hiding this comment.
@codingtwinky thanks for updating the README. If I may pick nits, you missed one little bit doesn't --> don't + remove the s in assumes.
FredrikNoren
reviewed
Apr 1, 2017
CHANGELOG.md
Outdated
| @@ -7,6 +7,9 @@ Use the following format for additions: ` - VERSION: [feature/patch (if applicab | |||
| - Fix cli arguments [#871](https://github.com/FredrikNoren/ungit/pull/871) | |||
| - Stop if ~/.ungitrc contains syntax error | |||
| - Removed official support ini format of ~/.ungitrc, because internal API supports only JSON | |||
| - Retain commit messages when commit fails [#882](https://github.com/FredrikNoren/ungit/pull/882) | |||
There was a problem hiding this comment.
1.1.11 is already out right? Think you ment to cut a new version here
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.