Skip to content

[Reverse String] Clean up approach docs#4226

Merged
BethanyG merged 4 commits into
exercism:mainfrom
Yrahcaz7:reverse-string-approach-cleanup
May 31, 2026
Merged

[Reverse String] Clean up approach docs#4226
BethanyG merged 4 commits into
exercism:mainfrom
Yrahcaz7:reverse-string-approach-cleanup

Conversation

@Yrahcaz7
Copy link
Copy Markdown
Contributor

@Yrahcaz7 Yrahcaz7 commented May 30, 2026

As discussed here.

Unfortunately, one line of performance/snippet.md had to be cut off to make the table render properly while keeping it within the 8-line limit.

Side note: Maybe there should be a doc somewhere that says how to link to headers in pages on the Exercism website? There was one in these docs, but it didn't work. I had to convert invalid characters to hyphens add an "h-" prefix to fix it: #h-using-map-and-lambbda-with-join-instead-of-a-loop.

@github-actions

This comment was marked as resolved.

@github-actions github-actions Bot closed this May 30, 2026
@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

@BethanyG, I added another commit, but it isn't showing up. Will it get auto-added when the PR is reopened, or do I need to do some git weirdness?

@BethanyG
Copy link
Copy Markdown
Member

@BethanyG, I added another commit, but it isn't showing up. Will it get auto-added when the PR is reopened, or do I need to do some git weirdness?

Apologies. Just getting to this (busy morning). That is very strange. Did you push your change to your fork? That should have been picked up automatically. Also make sure you were on your local branch. I cannot tell you how many times I've made an addition to something and then realized I'd switched branches (😱). But it also might be the PR state. I will reopen and we can check that now.

@BethanyG BethanyG reopened this May 30, 2026
@BethanyG
Copy link
Copy Markdown
Member

Side note: Maybe there should be a doc somewhere that says how to link to headers in pages on the Exercism website? There was one in these docs, but it didn't work. I had to convert invalid characters to hyphens add an "h-" prefix to fix it: #h-using-map-and-lambbda-with-join-instead-of-a-loop.

🤔 Wonder if there is a way we can test this by (maybe) doing a test approach with a bunch of said links? And then we can update the docs, or propose a change to them. Lemme go dig and see if I can find the Exercism docs in question. The likely path would be to propose the changes on the forum. So:

  1. Open a test PR (approach, hint file, instruction append file...) with header links both ways to test.
  2. Once we figure it out, propose a change to the Exercism docs to alert to the correct format on the forum
  3. PR the change (if you get the go-ahead).

@BethanyG
Copy link
Copy Markdown
Member

Try the push again to see if it shows up now, Otherwise, we have some git-wierdness to deal with. Sorry.

@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

Yrahcaz7 commented May 30, 2026

It seems like the commit appeared after the PR was re-opened.


Open a test PR (approach, hint file, instruction append file...) with header links both ways to test.

Is that necessary? I think you can see the correct format by inspecting the HTML of any page on the Exercism website. (That's how I figured it out, at least.)

Example:

Screenshot 2026-05-30 at 3 43 26 PM

Edit: Just realized that it still won't work after the PR because "lambbda" will be changed to "a lambda". Let me fix that real quick...

@BethanyG
Copy link
Copy Markdown
Member

So the reason I want to test this is because there is one scenario where you are linking to something in the same doc, and one where you are linking to something in a different doc. IIRC, there was some sort of fix needed for Anchor Links - and I haven't found it yet. So there might be two different forms. Let me dig a bit.

@BethanyG
Copy link
Copy Markdown
Member

So this is the PR in question, and this is now my question: Since this says "automatically", is this part of processing, and so we shouldn't put the h- in because the code will do it twice, or do we need to put it in?

@BethanyG
Copy link
Copy Markdown
Member

Looking at it closer, it does appear that we need to do that for the Anchor Links (prepend h-). And if so, we certainly need to change/amend this doc. But someone more fluent with Ruby and/or linking to Anchor Links needs to weigh in.

@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

Should I start a thread on the forum about this then?

@BethanyG
Copy link
Copy Markdown
Member

BethanyG commented May 30, 2026

Yeah - would you? Thank you 💙. Make sure to link to that PR, as well as the approach and what you saw in the JS terminal. And maybe ask other maintainers what they've done to link to an Anchor on a page.

And I do think the Markdown Spec needs editing, as well and perhaps the Style Guide?

I don't wanna bug Erik too much, but this is probably an area where he has more expertise than the rest of us.

@BethanyG
Copy link
Copy Markdown
Member

BethanyG commented May 30, 2026

HOLD UP! Should have searched the forum better. 😅

This discusses Anchor Links. And here is an additional PR as a fix.....

Still doesn't have any explicit documentation, tho. And I think we need some — bot "automatic" (use the copy buttons) and "manual" (prepend h- to the section header in the URL). 😄

@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

It seems like the "automatic" way only works on the contributing docs (and maybe some other parts of the website), but not on approach docs.

@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

Somewhat of a tangent, but it seems the style guide says:

Use the "Oxford Comma" (also known as the Serial Comma) in lists. For example, rather than "I love my parents, Lady Gaga and Humpty Dumpty", write "I love my parents, Lady Gaga, and Humpty Dumpty".

I have been using it when writing new approaches, but now I realize I missed some of those in the typo patrol...

@BethanyG
Copy link
Copy Markdown
Member

I have been using it when writing new approaches, but now I realize I missed some of those in the typo patrol...

Let's start a new issue for Oxford Commas. 😄 And we can loop back and take care of them afterwards (in all the docs where we missed it). That way, we can save the approaches audit from stalling in a recursive loop. 😂

We could also potentially audit both the track docs and the repo docs. But we should do that in its own issue as well.

Copy link
Copy Markdown
Member

@BethanyG BethanyG left a comment

Choose a reason for hiding this comment

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

Wow. That was a lot of work! Bumping the Rep on this. 😄

I had to work pretty hard to find anything worth requesting change on. And it was sorta a stretch. The only thing I couldn't via suggest was to go back and do a sweep of in-line links, and make sure we use reflinks instead. Buuut. The more I think about it, the more I think we should do a new issue for reflinks, and not weigh this particular fix down with that. 😄

Comment thread exercises/practice/reverse-string/.approaches/additional-approaches/content.md Outdated
Comment thread exercises/practice/reverse-string/.approaches/built-in-reversed/content.md Outdated
Comment thread exercises/practice/reverse-string/.approaches/list-and-join/content.md Outdated
Comment thread exercises/practice/reverse-string/.articles/performance/code/Benchmark.py Outdated
Comment thread exercises/practice/reverse-string/.articles/performance/code/Benchmark.py Outdated
@BethanyG BethanyG added the x:rep/large Large amount of reputation label May 30, 2026
Co-authored-by: BethanyG <BethanyG@users.noreply.github.com>
@Yrahcaz7 Yrahcaz7 requested a review from BethanyG May 30, 2026 22:14
@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

@BethanyG, I'm a little confused as to what the earlier 👎 means... Does the "automatic" way work for you on the approach docs? I tried on both Safari and Firefox, and both times I didn't see any link button when hovering over the area where it should be.

@BethanyG
Copy link
Copy Markdown
Member

@BethanyG, I'm a little confused as to what the earlier 👎 means... Does the "automatic" way work for you on the approach docs? I tried on both Safari and Firefox, and both times I didn't see any link button when hovering over the area where it should be.

Sorry! The 👎🏽 was a "BOOO" reaction to the situation — not disagreement. It doesn't work for me either, and should. But we should also have docs about it. THANK YOU btw for opening a forum issue. 😄

@BethanyG
Copy link
Copy Markdown
Member

Are we ready to merge?

@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

🤔 I'm conflicted between waiting to get responses on the forum post vs just pushing it and seeing if it works. Thoughts?

@BethanyG
Copy link
Copy Markdown
Member

🤔 I'm conflicted between waiting to get responses on the forum post vs just pushing it and seeing if it works. Thoughts?

On one hand, it was broken before — don't see how it could be more broken. On the other? A discussion might be good, although I suspect most folx involved would not have encountered the situation, and would have absolutely no idea how to solve it (apart from Aaron and Erik who made the changes in the first place).

@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

I guess it would be better to just merge and test if it works, then.

@BethanyG BethanyG merged commit 530d9a8 into exercism:main May 31, 2026
7 checks passed
@Yrahcaz7 Yrahcaz7 deleted the reverse-string-approach-cleanup branch May 31, 2026 00:16
@Yrahcaz7
Copy link
Copy Markdown
Contributor Author

It seems like it works! (At first I thought that it didn't, but of course it was just a caching issue)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

x:rep/large Large amount of reputation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants