Skip to content

Conversation

@Trott
Copy link
Member

@Trott Trott commented Aug 10, 2016

Checklist
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

doc

Description of change

POST_STATUS_TO_PR did not used to work.

Now it works.

Update the onboarding documentation accordingly.

@Trott Trott added the doc Issues and PRs related to the documentations. label Aug 10, 2016
@addaleax
Copy link
Member

Commit message typo: did not used to

LGTM but got a question… does it ever make sense not to check that box?

@Trott
Copy link
Member Author

Trott commented Aug 10, 2016

@addaleax (Whoops, ignore previous comment, I misread your question.) I guess not. I suppose it might be worthwhile asking @nodejs/build to make that box checked by default and then we don't have to mention it at all in the onboarding doc!

`POST_STATUS_TO_PR` previously did not work. Now it works. Update the onboarding documentation accordingly.
@Trott
Copy link
Member Author

Trott commented Aug 10, 2016

@addaleax Updated the commit message. Thanks.

@Fishrock123
Copy link
Contributor

@jbergstroem think we are ready to move forward with this or is there still stuff being worked on there?

@fhinkel
Copy link
Member

fhinkel commented Aug 11, 2016

Can we change the default for POST_STATUS_TO_PR to be checked? Seems easier than to add an extra instruction to the onboarding doc about actively turning it on.

@jasnell
Copy link
Member

jasnell commented Aug 11, 2016

Yeah, I'd definitely prefer for post status to be on by default.

@jbergstroem
Copy link
Member

I'd like to get some more time working with this if that's ok. ETA ~1w.

@mhdawson
Copy link
Member

Same goes for me for having in on by default once we are ready.

Trott added a commit to Trott/io.js that referenced this pull request Aug 12, 2016
`POST_STATUS_TO_PR` previously did not work. Now it works. Update the onboarding documentation accordingly. PR-URL: nodejs#8059 Reviewed-By: Anna Henningsen <anna@addaleax.net>
@Trott
Copy link
Member Author

Trott commented Aug 12, 2016

Landed in fa1476c.

(We can update it again when the default switches to the box being checked. I expect to be updating the onboarding doc frequently anyway.)

@Trott Trott closed this Aug 12, 2016
cjihrig pushed a commit that referenced this pull request Aug 15, 2016
`POST_STATUS_TO_PR` previously did not work. Now it works. Update the onboarding documentation accordingly. PR-URL: #8059 Reviewed-By: Anna Henningsen <anna@addaleax.net>
@cjihrig cjihrig mentioned this pull request Aug 15, 2016
MylesBorins pushed a commit that referenced this pull request Sep 9, 2016
`POST_STATUS_TO_PR` previously did not work. Now it works. Update the onboarding documentation accordingly. PR-URL: #8059 Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Sep 28, 2016
`POST_STATUS_TO_PR` previously did not work. Now it works. Update the onboarding documentation accordingly. PR-URL: #8059 Reviewed-By: Anna Henningsen <anna@addaleax.net>
rvagg pushed a commit that referenced this pull request Oct 18, 2016
`POST_STATUS_TO_PR` previously did not work. Now it works. Update the onboarding documentation accordingly. PR-URL: #8059 Reviewed-By: Anna Henningsen <anna@addaleax.net>
MylesBorins pushed a commit that referenced this pull request Oct 26, 2016
`POST_STATUS_TO_PR` previously did not work. Now it works. Update the onboarding documentation accordingly. PR-URL: #8059 Reviewed-By: Anna Henningsen <anna@addaleax.net>
@MylesBorins MylesBorins mentioned this pull request Oct 26, 2016
@Trott Trott deleted the itsalive111 branch January 13, 2022 22:44
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.

8 participants