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: alphabetize all.markdown #566

Closed

Conversation

brendanashworth
Copy link
Contributor

Previously the order made no sense, especially since toc.markdown was
alphabetized but all.markdown was not.

This might fix issue #393 but after PR #553, I don't think we're on the
same page.

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

The docs appear to alphabetize anyways; does this affect output at all?

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Jan 23, 2015
@brendanashworth
Copy link
Contributor Author

@Fishrock123 the list was already alphabetized, but the "all" page was not - this pull request changes the all page, nothing else. I should have clarified that.

@Fishrock123
Copy link
Contributor

OH.

LGTM, @chrisdickinson?

@ralphtheninja
Copy link
Contributor

👍

@chrisdickinson
Copy link
Contributor

LGTM!

@MattMS
Copy link

MattMS commented Jan 24, 2015

Sorry @brendanashworth, I misunderstood your comment in #393.

Am I right in understanding that there is no interest in reordering pages as in #553?

@brendanashworth
Copy link
Contributor Author

@MattMS it wasn't your misunderstanding - I previously thought your intent was the same as illustrated in this PR, but then you opened #553 and I realized we probably didn't have the same intention, so I split what I had in mind into this PR and chose to let you continue with your idea.

I'm not going to say that there is no interest - you do have solid reasoning behind it. I wouldn't drop it unless a strong reason to not do so is brought up.

Fishrock123 pushed a commit that referenced this pull request Jan 24, 2015
Previously `toc.markdown` was alphabetized but `all.markdown` was not.

PR-URL: #566
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
@MattMS
Copy link

MattMS commented Jan 24, 2015

Thanks @brendanashworth, sounds like a good plan 😄
I'll leave #393 and #553 as-is and see if they gather more comments.

This PR is a great start anyway!

@Fishrock123
Copy link
Contributor

Fixed in 35a4f11 - thanks!

This was referenced Dec 1, 2023
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.

5 participants