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

Reorder fs doc #553

Closed
wants to merge 1 commit into from
Closed

Reorder fs doc #553

wants to merge 1 commit into from

Conversation

MattMS
Copy link

@MattMS MattMS commented Jan 22, 2015

Reordered the functions alphabetically, but kept the matching "Sync" functions with their async partners.

Left the classes at the end of the file, but ordered them alphabetically too.

@MattMS MattMS mentioned this pull request Jan 22, 2015
@bnoordhuis
Copy link
Member

I have no opinion on whether this is a good or a bad change, I just want to point out that it makes automatic merging from joyent/node pretty much impossible. Not the end of the world, just something to keep in mind.

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Jan 22, 2015
@MattMS
Copy link
Author

MattMS commented Jan 22, 2015

Considering it only took 10 minutes or so, I'm not worried if this gets dropped for now.
I would think the merging would take priority.

Any suggestions on a cleaner way to do it?

brendanashworth added a commit to brendanashworth/io.js that referenced this pull request Jan 23, 2015
Previously the order made no sense, especially since `toc.markdown` was
alphabetized but `all.markdown` was not.

This might fix issue nodejs#393 but after PR nodejs#553, I don't think we're on the
same page.
@Fishrock123
Copy link
Contributor

Maybe this should be pulled against joynet/node first?

@MattMS
Copy link
Author

MattMS commented Jan 31, 2015

I'm happy to make up a new pull request there as long as there is interest in the reordering.

@Fishrock123
Copy link
Contributor

You might want to bring up the issue there. It would be easier to migrate up, i think.

@MattMS
Copy link
Author

MattMS commented Jan 31, 2015

Sounds good. I'll open an issue similar to #393 and check the feedback.

@cjihrig
Copy link
Contributor

cjihrig commented Feb 3, 2015

Closing this until we see how things play out on joyent/node.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants