Skip to content
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

doc: special cases in http module response body #8992

Closed
wants to merge 1 commit into from

Conversation

minervapanda
Copy link

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines

Affected core subsystem(s)

http

Description of change

Response body is omited when there is a HEAD request , 204 or 304 response. Solves #8057

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Oct 9, 2016
@minervapanda
Copy link
Author

How do I handle previous commits even though I changed the branch for new pr. @addaleax

@addaleax
Copy link
Member

addaleax commented Oct 9, 2016

@minervapanda Oh, so… I guess what you did was switching branches to pr2 after you were done with #8991, and want to remove the url.resolve() commit here?

So, the steps for that would be:

  • (If you haven’t set up an upstream remote or aren’t sure, do git remote add upstream https://github.com/nodejs/node.git)
  • git fetch upstream
  • Make sure you’re on the right branch, e.g. by checking that git log tells you the most recent commits and includes the ones you want to remove.

Then: git rebase -i upstream/master. When trying that command on your PR, the command opens an editor window, in which I can read these two lines:

pick 4c58202b1957 doc: url.resolve() method's behaviour
pick 89211a6a2558 doc: response body in http module

What you’re editing there are basically commands to be executed by git in order to re-create a modified version of your branch. If you leave the file as-is and just save it, the 2 commits listed there will just be used (“picked”) to re-write your branch the way it was before.

If you delete the first line (in nano, that’s Ctrl+K) and save the file by closing the editor (in nano, that’s Ctrl+X), git only picks the second commit and drops the first one, so that only the changes that you made on your own distinguish your branch from master.

After that, check git log again to make sure the first commit has been removed.

If you’re somewhat certain that everything looks the way you want it to look like, use git push --force-with-lease origin pr2 to fix up your PR.

(If something goes wrong or you’re unsure about anything – again: don’t be shy to ask, here or e.g. in #node-dev on Freenode.)

@lpinca
Copy link
Member

lpinca commented Oct 9, 2016

@minervapanda I think you got the "url" commit here because you created the "pr2" branch from "pr1".

From "pr1" switch back to master and create "pr2" from there.

Hope this makes sense.

@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Oct 9, 2016
@rvagg rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@minervapanda
Copy link
Author

@lpinca When can this be merged to the master ?

@lpinca
Copy link
Member

lpinca commented Nov 2, 2016

@minervapanda can you please remove the "url" changes from this pr?

@jasnell
Copy link
Member

jasnell commented Nov 2, 2016

Agree with @lpinca .. the url changes should either be put into a separate commit in this PR or moved into a separate PR altogether.

@jasnell jasnell added the stalled Issues and PRs that are stalled. label Mar 1, 2017
@fhinkel
Copy link
Member

fhinkel commented May 26, 2017

There hasn't been any activity here. I'm closing this. Feel free to reopen (or ping a collaborator) if I closed this in error.

@fhinkel fhinkel closed this May 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. http Issues or PRs related to the http subsystem. stalled Issues and PRs that are stalled.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants