Skip to content

Make minor grammar corrections/updates to async/promise-chaining #1661

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

Merged
merged 2 commits into from
Jan 2, 2020

Conversation

jchue
Copy link
Contributor

@jchue jchue commented Dec 8, 2019

Split out and revised changes originally in #1612

@lex111 lex111 requested a review from paroche December 19, 2019 20:16
Copy link
Collaborator

@paroche paroche left a comment

Choose a reason for hiding this comment

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

All changes OK. I made a couple suggestions for pretty minor improvements.

@iliakan
Copy link
Member

iliakan commented Dec 27, 2019

@paroche should we merge it in its current state?

@paroche
Copy link
Collaborator

paroche commented Dec 27, 2019

Would be OK to merge as is. I had some small suggestions but they could be done later, or not at all.

When you "assign" it to me, what exactly does that mean?

@iliakan
Copy link
Member

iliakan commented Dec 27, 2019

@paroche that means I'm unable to resolve the matter, as it's not about JavaScript, but rather about the English language, the wording and the syntax.

So I kindly ask you to take a look, request changes and merge if you deem appropriate :)

@paroche
Copy link
Collaborator

paroche commented Dec 27, 2019

OK. Just seems like you've asked me to look at things before without "assigning" them to me. I guess it's the "merge if I deem appropriate" part that might be different.

Though with these more extensive changes where I agree with some, not with others, and for others might have a third (and/or forth) suggestion, I'm not sure how to proceed -- don't think I can push it piece-meal or make revisions (except after pushing it all). And may want to give the author of the PR a chance to make revisions or comments first. So then I just comment.

@iliakan
Copy link
Member

iliakan commented Dec 28, 2019

@paroche you use create a review and "request changes" using the button in the right-upper corner, right?

Then the PR author should review and update their PR.

@jchue
Copy link
Contributor Author

jchue commented Jan 2, 2020

Updated

@paroche paroche merged commit a09d162 into javascript-tutorial:master Jan 2, 2020
@paroche
Copy link
Collaborator

paroche commented Jan 2, 2020

OK. Too bad approving it piece-meal isn't an option (or doesn't seem to be). Seems like would be easier for long series of edits in one PR.

@jchue
Copy link
Contributor Author

jchue commented Jan 2, 2020

Agreed.

@paroche
Copy link
Collaborator

paroche commented Jan 2, 2020

That was in response to Ilya's last comment, but I'm glad you agree. Is harder to keep track this way and it can lead to a lot of redundancy in reviewing changes.

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