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

Markup reference #824

Merged
merged 18 commits into from Jul 11, 2022
Merged

Markup reference #824

merged 18 commits into from Jul 11, 2022

Conversation

BurdetteLamar
Copy link
Member

This PR is proposed as a replacement for the section "RDoc Markup Reference" in the documentation for class RDoc::Markup. The PR covers the topics found there.

I've built this as the separate "dummy" class RDoc::MarkupReference to allow examples of on-page links and references. This avoids adding illustrative dummy code in the class RDoc::Markup itself.

There are some specific questions and requests for reviewers, specifically where:

  • I thought that the RDoc behavior may be wrong.
  • I was not able to craft an example as I'd like.

I'm marking this as a draft at least until those issues are resolved.

Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

Seems strange to add this in a place where it could be required. Since this is only for documentation purposes, having this file under lib seems wrong.

I did a very brief review and don't see major issues with the text. I don't know the answer to any of the questions, though.

Note that I'm not an rdoc maintainer, and probably not a good person to review this.

@BurdetteLamar
Copy link
Member Author

Seems strange to add this in a place where it could be required. Since this is only for documentation purposes, having this file under lib seems wrong.

I did a very brief review and don't see major issues with the text. I don't know the answer to any of the questions, though.

Note that I'm not an rdoc maintainer, and probably not a good person to review this.

Thanks, @jeremyevans. Your ideas, as always, are helpful. I can move the file do new directory doc/,
which also requires citing it in the Rakefile. Does that make sense?

@BurdetteLamar
Copy link
Member Author

I have moved the dummy class MarkupReference from lib/ to new directory /doc, and have added the file to the Rakefile so that it will be built into the documentation.

@BurdetteLamar BurdetteLamar marked this pull request as ready for review July 19, 2021 19:16
Copy link
Member

@aycabta aycabta left a comment

Choose a reason for hiding this comment

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

Okay, I think this is a good patch. It's a very valuable document. However, there are too many commits, would you squash them into one?

@BurdetteLamar
Copy link
Member Author

BurdetteLamar commented Jul 26, 2021

Okay, I think this is a good patch. It's a very valuable document. However, there are too many commits, would you squash them into one?

Can I just use the "Squash and merge" button here?

BUT!! Before that, there are six questions and comments needing review. Search for "Reviewers:"

@BurdetteLamar
Copy link
Member Author

@aycabta, can you suggest someone to answer the "Reviewers:" questions embedded in this PR?

@BurdetteLamar BurdetteLamar marked this pull request as draft July 28, 2021 18:15
@BurdetteLamar BurdetteLamar marked this pull request as ready for review July 30, 2021 13:53
@BurdetteLamar
Copy link
Member Author

@aycabta, do you want this PR to be squashed and merged, even with its remaining questions unanswered?

@BurdetteLamar
Copy link
Member Author

@olleolleolle, thanks much!

@BurdetteLamar
Copy link
Member Author

So, @Team, how does this get finished up?

As I said in the original comment, this covers the same topics as in the old markup reference. There's much more that can be added -- topics not previously covered. I'm eager to continue, but first need to get this finished and merged.

@BurdetteLamar
Copy link
Member Author

I removed the 6 questions that were embedded in the text.

Now I find that I can no longer push to here:

remote: fatal error in commit_refs
To https://github.com/BurdetteLamar/rdoc.git
! [remote rejected] markup_reference -> markup_reference (failure)
error: failed to push some refs to 'https://github.com/BurdetteLamar/rdoc.git'

If no one objects, I'll close this and open another PR that will be ready to merge.

@BurdetteLamar
Copy link
Member Author

Oh, now it did allow me to push. @aycabta, ready to squash and merge?

@BurdetteLamar
Copy link
Member Author

@aycabta , still waiting for final approval (or disapproval).

nobu added a commit to nobu/rdoc that referenced this pull request Aug 28, 2021
nobu added a commit to nobu/rdoc that referenced this pull request Aug 28, 2021
@aycabta
Copy link
Member

aycabta commented Sep 3, 2021

Sorry, I think I'll get some time to review later this month. Give me a little longer.

@BurdetteLamar
Copy link
Member Author

Sorry, I think I'll get some time to review later this month. Give me a little longer.

Thanks, and no rush. Glad to know you're still interested.

nobu added a commit to nobu/rdoc that referenced this pull request Sep 10, 2021
matzbot pushed a commit to ruby/ruby that referenced this pull request Sep 11, 2021
@BurdetteLamar
Copy link
Member Author

@aycabta, can you review this PR? Or suggest someone else who can?

@BurdetteLamar
Copy link
Member Author

@aycabta, any progress on this?

@BurdetteLamar
Copy link
Member Author

@aycabta, there is much more that can be done with the RDoc documentation, but that can't be started until this PR is merged. Is there anything more that needs to be done here?

@BurdetteLamar
Copy link
Member Author

@aycabta, can this be reviewed by someone else? I'm thinking maybe @jeremyevans or @peterzhu2118. If not, where are we with this?

@BurdetteLamar
Copy link
Member Author

Somehow I thought that @aycabta is the maintainer for RDoc; now I see that the maintainers are @drbrain and @hsbt. Can one of you review this PR?

@BurdetteLamar
Copy link
Member Author

To avoid confusion, note that the "six questions" mentioned above are moot, and no longer relevant.

@BurdetteLamar BurdetteLamar requested a review from hsbt March 17, 2022 17:18
@BurdetteLamar
Copy link
Member Author

Okay, I think this is a good patch. It's a very valuable document. However, there are too many commits, would you squash them into one?

That's from @aycabta, many months ago, yet I can't get a review from any committer, not even the RDoc maintainers? I'm baffled.

@hsbt hsbt removed their request for review March 19, 2022 00:19
@hsbt
Copy link
Member

hsbt commented Mar 19, 2022

@BurdetteLamar see https://twitter.com/aycabta/status/1458494862202789893

Not everyone has enough time, motivation, and health for the all-time. Your rush request seems rude attitude.

@hsbt
Copy link
Member

hsbt commented Mar 19, 2022

Your contribution is valuable, I understand. After 1 year, there is no response from @aycabta . I will manage this.

Please wait a while.

@BurdetteLamar
Copy link
Member Author

Thanks, @hsbt.

doc/markup_reference.rb Outdated Show resolved Hide resolved
Co-authored-by: Adam Daniels <adam@mediadrive.ca>
@BurdetteLamar
Copy link
Member Author

Your contribution is valuable, I understand. After 1 year, there is no response from @aycabta . I will manage this.

Please wait a while.

@hsbt, may I have your permission to recruit reviewers for this PR?

@hsbt hsbt merged commit 92c04ce into ruby:master Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

6 participants