Skip to content
This repository has been archived by the owner on Aug 31, 2018. It is now read-only.

refactor README.md contents #1

Merged
merged 3 commits into from
Aug 22, 2017
Merged

refactor README.md contents #1

merged 3 commits into from
Aug 22, 2017

Conversation

ghost
Copy link

@ghost ghost commented Aug 21, 2017

this is a very rough edit that, for now, mainly just replaces all instances of Node.js with Ayo.js in the README file. obviously, most of the readme is broken right now, so this PR can serve as a general place to refactor the readme as far as we can.

another alternative would be to just drop the current readme altogether and draft a new one. what does everyone think?

@ghost
Copy link
Author

ghost commented Aug 21, 2017

additionally, almost all of the links point to the current website right now. how big of a priority should a new website be?

@ghost ghost changed the title doc: replaced node.js with ayo.js in README.md refactor README.md contents Aug 21, 2017
addaleax
addaleax approved these changes Aug 21, 2017
View reviewed changes
README.md Outdated
@@ -464,7 +446,7 @@ maintaining the Node.js project.

### Release Team

Node.js releases are signed with one of the following GPG keys:
Ayo.js releases are signed with one of the following GPG keys:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don’t think this is very accurate :) How about just replacing are signed → were signed for now?

varjmes
varjmes reviewed Aug 22, 2017
View reviewed changes
Copy link

@varjmes varjmes left a comment

Choose a reason for hiding this comment

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

additionally, almost all of the links point to the current website right now. how big of a priority should a new website be?

I'd say not a priority at all, right now.

README.md Outdated
@@ -464,7 +446,7 @@ maintaining the Node.js project.

### Release Team

Node.js releases are signed with one of the following GPG keys:
Ayo.js releases were signed with one of the following GPG keys:
Copy link

Choose a reason for hiding this comment

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

They never were. I think we should get rid of this section or say "Ayo.js is forked from Node.js, and their releases were signed with..."

Copy link

Choose a reason for hiding this comment

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

I realise that this PR is currently a find and replace, but I think my above comment is worth doing now.

README.md Outdated
@@ -509,7 +491,7 @@ Previous releases may also have been signed with one of the following GPG keys:

### Working Groups

Information on the current Node.js Working Groups can be found in the
Information on the current Ayo.js Working Groups can be found in the
Copy link

Choose a reason for hiding this comment

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

No, they can't. Because we don't have any yet. :) I'd remove the section.

Copy link

Choose a reason for hiding this comment

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

I realise that this PR is currently a find and replace, but I think my above comment is worth doing now.

varjmes
varjmes reviewed Aug 22, 2017
View reviewed changes
README.md
@@ -175,7 +157,7 @@ handling your report.

## Current Project Team Members
Copy link

Choose a reason for hiding this comment

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

Can we scrap this list?

@varjmes
Copy link

varjmes commented Aug 22, 2017

I'm gonna stop commenting now, but I say we Remove text from all of the subheadings we haven't dealt with (most of them) and Put a "to be written" under each. Otherwise this PR will be open forever and I'm a big fan of small iterations.

@ghost
Copy link
Author

ghost commented Aug 22, 2017

@varjmes done!

@varjmes varjmes merged commit aff4e72 into master Aug 22, 2017
@varjmes varjmes deleted the adjust-readme branch August 22, 2017 14:41
@varjmes
Copy link

varjmes commented Aug 22, 2017

First PR to Ayo.js, merged!

bitmoji

@varjmes varjmes mentioned this pull request Aug 22, 2017
Merged
@addaleax
Copy link
Contributor

By the way, if we want to stick to a release process similar to that of Node, it would be not mandatory but helpful if we kept its commit metadata rules to some degree

Not saying we have to :)

@ghost
Copy link
Author

ghost commented Aug 22, 2017

@addaleax you mean squashing PRs? i have no idea how it works in node core, to be honest

@addaleax
Copy link
Contributor

When we merge PR we add metadata in the style of what you see in nodejs/node@6a0e3b1 (PR-URL: is the full URL, not just the #, github just doesn’t display it)

In particular, PR-URL is required by the current tooling for making releases/updating branches

@ghost
Copy link
Author

ghost commented Aug 22, 2017

oh, okay! should we squash/reword the commits that are now on master, then?

@addaleax
Copy link
Contributor

I would suggest doing that until we agree that we want some other kind of format :)

(There’s even tooling like https://github.com/evanlucas/core-validate-commit for checking that data)

@aqrln
Copy link
Contributor

aqrln commented Aug 22, 2017

@pup regarding the metadata, basically, what Anna said, but I'd add there is some documentation about it (https://github.com/ayojs/ayo/blob/master/COLLABORATOR_GUIDE.md#landing-pull-requests) that describes both these policies and some related Git recipes which might be helpful :)

Speaking about whether these guidelines should be changed, both leaving and omitting the Reviewed-By fields would work (though they may become handy since GitHub may not be there forever), but PR-URL definitely must be a thing, IMHO. Not only it is used by the release tooling, but I also find it extremely useful when bisecting, debugging or just exploring and tweaking code to see how it works if you can follow a URL and see additional context and discussion.

@varjmes
Copy link

varjmes commented Aug 22, 2017

Thank you @addaleax and @aqrln :)

@aqrln
Copy link
Contributor

aqrln commented Aug 22, 2017

oh, okay! should we squash/reword the commits that are now on master, then?

I'd say this repository is currently in position where doing git rebase -i on the new commits on master won't cause any harm yet, so yes, it is worth doing so. @addaleax how do you think?

@addaleax
Copy link
Contributor

@aqrln Yeah, totally agree, esp with 0 open PRs. :) I can do that if nobody else is feeling like it.

@varjmes
Copy link

varjmes commented Aug 22, 2017

i would greatly appreciate someone else doing it, so I can take note :)

addaleax pushed a commit that referenced this pull request Aug 22, 2017
@addaleax
doc: replaced node.js with ayo.js in README.md
fed7d02 
PR-URL: #1
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@addaleax
Copy link
Contributor

@varjmes @pup Okay, I went ahead and squashed/reworded so the commits are fed7d02, 69b8f88. This would be the typical format that Node currently uses. :)

ayo

addaleax pushed a commit that referenced this pull request Oct 26, 2017
@danbev @addaleax
test: allow tests to pass without internet
479f41f 
Currently when running the test without an internet connection there are
two JavaScript test failures and one cctest. The cctest only fails on
Mac as far as I know. (I've only tested using Mac and Linux thus far).

This commit moves the two JavaScript tests to test/internet.

The details for test_inspector_socket_server.cc:

[ RUN      ] InspectorSocketServerTest.FailsToBindToNodejsHost
make[1]: *** [cctest] Segmentation fault: 11
make: *** [test] Error 2

lldb output:

[ RUN      ] InspectorSocketServerTest.FailsToBindToNodejsHost
Process 63058 stopped
* thread #1: tid = 0x7b175, 0x00007fff96d04384
* libsystem_info.dylib`_gai_simple + 87, queue =
* 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1,
* address=0x0)
    frame #0: 0x00007fff96d04384 libsystem_info.dylib`_gai_simple + 87
libsystem_info.dylib`_gai_simple:
->  0x7fff96d04384 <+87>: movw   (%rdx), %ax
    0x7fff96d04387 <+90>: movw   %ax, -0x2a(%rbp)
    0x7fff96d0438b <+94>: movq   %r13, -0x38(%rbp)
    0x7fff96d0438f <+98>: movq   0x18(%rbp), %rcx

(lldb) bt
* thread #1: tid = 0x7b175, 0x00007fff96d04384
* libsystem_info.dylib`_gai_simple + 87, queue =
* 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1,
* address=0x0)
  * frame #0: 0x00007fff96d04384 libsystem_info.dylib`_gai_simple + 87
    frame #1: 0x00007fff96cfe98b libsystem_info.dylib`search_addrinfo +
179
    frame #2: 0x00007fff96cfafef libsystem_info.dylib`si_addrinfo + 2255
    frame #3: 0x00007fff96cfa67b libsystem_info.dylib`getaddrinfo + 179
    frame #4: 0x00000001017d8888
cctest`uv__getaddrinfo_work(w=0x00007fff5fbfe210) + 72 at
getaddrinfo.c:102
    frame #5: 0x00000001017d880e
cctest`uv_getaddrinfo(loop=0x000000010287cb80, req=0x00007fff5fbfe1c8,
cb=0x0000000000000000, hostname="nodejs.org", service="0",
hints=0x00007fff5fbfe268) + 734 at getaddrinfo.c:192
    frame #6: 0x000000010171f781
cctest`node::inspector::InspectorSocketServer::Start(this=0x00007fff5fbfe658)
+ 801 at inspector_socket_server.cc:398
    frame #7: 0x00000001016ed590
cctest`InspectorSocketServerTest_FailsToBindToNodejsHost_Test::TestBody(this=0x0000000105001fd0)
+ 288 at test_inspector_socket_server.cc:593

I'm not sure about the exact cause for this but when using a standalone
c program to simulate this it seems like when the ai_flags
`AI_NUMERICSERV` is set, which is done in inspector_socket_server.cc
line 394, the servname (the port in the FailsToBindToNodejsHost test) is
expected to be a numeric port string to avoid looking it up in
/etc/services. When the port is 0 as is it was before this commit the
segment fault occurs but not if it is non-zero.

PR-URL: nodejs/node#16255
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit that referenced this pull request Dec 7, 2017
@danbev @addaleax
test: allow tests to pass without internet
1468161 
Currently when running the test without an internet connection there are
two JavaScript test failures and one cctest. The cctest only fails on
Mac as far as I know. (I've only tested using Mac and Linux thus far).

This commit moves the two JavaScript tests to test/internet.

The details for test_inspector_socket_server.cc:

[ RUN      ] InspectorSocketServerTest.FailsToBindToNodejsHost
make[1]: *** [cctest] Segmentation fault: 11
make: *** [test] Error 2

lldb output:

[ RUN      ] InspectorSocketServerTest.FailsToBindToNodejsHost
Process 63058 stopped
* thread #1: tid = 0x7b175, 0x00007fff96d04384
* libsystem_info.dylib`_gai_simple + 87, queue =
* 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1,
* address=0x0)
    frame #0: 0x00007fff96d04384 libsystem_info.dylib`_gai_simple + 87
libsystem_info.dylib`_gai_simple:
->  0x7fff96d04384 <+87>: movw   (%rdx), %ax
    0x7fff96d04387 <+90>: movw   %ax, -0x2a(%rbp)
    0x7fff96d0438b <+94>: movq   %r13, -0x38(%rbp)
    0x7fff96d0438f <+98>: movq   0x18(%rbp), %rcx

(lldb) bt
* thread #1: tid = 0x7b175, 0x00007fff96d04384
* libsystem_info.dylib`_gai_simple + 87, queue =
* 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1,
* address=0x0)
  * frame #0: 0x00007fff96d04384 libsystem_info.dylib`_gai_simple + 87
    frame #1: 0x00007fff96cfe98b libsystem_info.dylib`search_addrinfo +
179
    frame #2: 0x00007fff96cfafef libsystem_info.dylib`si_addrinfo + 2255
    frame #3: 0x00007fff96cfa67b libsystem_info.dylib`getaddrinfo + 179
    frame #4: 0x00000001017d8888
cctest`uv__getaddrinfo_work(w=0x00007fff5fbfe210) + 72 at
getaddrinfo.c:102
    frame #5: 0x00000001017d880e
cctest`uv_getaddrinfo(loop=0x000000010287cb80, req=0x00007fff5fbfe1c8,
cb=0x0000000000000000, hostname="nodejs.org", service="0",
hints=0x00007fff5fbfe268) + 734 at getaddrinfo.c:192
    frame #6: 0x000000010171f781
cctest`node::inspector::InspectorSocketServer::Start(this=0x00007fff5fbfe658)
+ 801 at inspector_socket_server.cc:398
    frame #7: 0x00000001016ed590
cctest`InspectorSocketServerTest_FailsToBindToNodejsHost_Test::TestBody(this=0x0000000105001fd0)
+ 288 at test_inspector_socket_server.cc:593

I'm not sure about the exact cause for this but when using a standalone
c program to simulate this it seems like when the ai_flags
`AI_NUMERICSERV` is set, which is done in inspector_socket_server.cc
line 394, the servname (the port in the FailsToBindToNodejsHost test) is
expected to be a numeric port string to avoid looking it up in
/etc/services. When the port is 0 as is it was before this commit the
segment fault occurs but not if it is non-zero.

PR-URL: nodejs/node#16255
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@varjmes @addaleax @aqrln