Skip to content

Conversation

@eljefedelrodeodeljefe
Copy link
Contributor

Checklist
  • tests and code linting passes
  • the commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

adds me to the readme.md as collaborator

@ChALkeR
Copy link
Member

LGTM, welcome!

@ChALkeRChALkeR added the doc Issues and PRs related to the documentations. label Apr 26, 2016
@evanlucas
Copy link
Contributor

LGTM. Welcome aboard

@santigimeno
Copy link
Member

LGTM and welcome!

@eljefedelrodeodeljefe
Copy link
ContributorAuthor

eljefedelrodeodeljefe commented Apr 26, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/2395/

Gonna merge this after CI :) Thanks guys

eljefedelrodeodeljefe added a commit that referenced this pull request Apr 26, 2016
PR-URL: #6389 Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
@eljefedelrodeodeljefe
Copy link
ContributorAuthor

Landed in be5d699

@eljefedelrodeodeljefeeljefedelrodeodeljefe deleted the doc/add-eljefedelrodeodeljefe-to-collaborators branch April 26, 2016 06:16
@eljefedelrodeodeljefe
Copy link
ContributorAuthor

With the process in the collaborators guide the PRs won't be marked as merged on GitHub right? Guess it doesn't matter too much.

@Trott
Copy link
Member

@eljefedelrodeodeljefe That's right. @evanlucas told me and others how to make it so that it shows as merged but I don't remember what it was.

@ChALkeR
Copy link
Member

ChALkeR commented Apr 26, 2016

@eljefedelrodeodeljefe@Trott
Trivial — just make sure your branch is rebased on master and push the final commits with all the modifications (including the PR-URL, Reviewed-By, etc) to your branch before landing it to master.

GitHub will label the PR as merged when it sees that all the commits with identical ids are landed on the target branch.

@eljefedelrodeodeljefe
Copy link
ContributorAuthor

Will try that next time then. Thought I did all that. Thanks

@jbergstroem
Copy link
Member

@ChALkeR we should really add that to the collaborators guide -- it makes it easier to make the assumption that the last commit(s) in the PR was also the one's merged.

@jbergstroem
Copy link
Member

@eljefedelrodeodeljefe you have to force push to your branch since you're rewriting history (git push -f)

@ChALkeR
Copy link
Member

@eljefedelrodeodeljefe The branch had 3f3f41a, the merged commit was be5d699. Those were not identical =).

Update: sorry, copy-pasted wrong commit id. Fixed.

@eljefedelrodeodeljefe
Copy link
ContributorAuthor

eljefedelrodeodeljefe commented Apr 26, 2016

Yep, after @jbergstroem comments I was realizing that. After adding metadata it needs another force push to the fork.

jasnell pushed a commit that referenced this pull request Apr 26, 2016
PR-URL: #6389 Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
eljefedelrodeodeljefe added a commit that referenced this pull request May 19, 2016
Git logs print my full name Robert Jefe Lindstaedt. When I did #6389 I forgot simply forgot it. PR-URL: #6880 Reviewed-By: Ben Noordhuis <[email protected]>
Fishrock123 pushed a commit that referenced this pull request May 23, 2016
Git logs print my full name Robert Jefe Lindstaedt. When I did #6389 I forgot simply forgot it. PR-URL: #6880 Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 1, 2016
PR-URL: #6389 Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
rvagg pushed a commit that referenced this pull request Jun 2, 2016
Git logs print my full name Robert Jefe Lindstaedt. When I did #6389 I forgot simply forgot it. PR-URL: #6880 Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 3, 2016
Git logs print my full name Robert Jefe Lindstaedt. When I did #6389 I forgot simply forgot it. PR-URL: #6880 Reviewed-By: Ben Noordhuis <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jun 24, 2016
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
PR-URL: #6389 Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
Git logs print my full name Robert Jefe Lindstaedt. When I did #6389 I forgot simply forgot it. PR-URL: #6880 Reviewed-By: Ben Noordhuis <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
PR-URL: #6389 Reviewed-By: Сковорода Никита Андреевич <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Santiago Gimeno <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jun 24, 2016
Git logs print my full name Robert Jefe Lindstaedt. When I did #6389 I forgot simply forgot it. PR-URL: #6880 Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docIssues and PRs related to the documentations.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@eljefedelrodeodeljefe@ChALkeR@evanlucas@santigimeno@Trott@jbergstroem