-
Notifications
You must be signed in to change notification settings - Fork 38
Fixes for PR 57 #58
base: master
Are you sure you want to change the base?
Fixes for PR 57 #58
Conversation
with open source blurb.
This allows users to copy text without copying the $.
Replaces certain code tags that do not need to show a $ with a mono-highlight span tag. Changes New Open Source section wording.
I wondered if it wouldn't be better to have a class called |
The areas where code was being used to highlight things weren't really code blocks though. The areas that I wrapped in span were mostly highlighting variable names, keywords, or file names such as: * <span class="mono-highlight">.sdkmanrc</span>
* <span class="mono-highlight">java_home</span>
* <span class="mono-highlight">sdkReleaseVersion</span> I think keeping the Instead of using span with a class I could use the |
I've pulled down your branch and run it locally and still see lots of dollars all over where they don't belong. I've taken care to clear my browser cache but that wasn't it either. Also, when I try to use the span tags with the For now, I think I'm going to roll back the changes as things are getting too hairy for me. If you feel up to it, I suggest raising a new PR with all your changes together in one go. I'm sorry about that as I know you've already spent substantial time on this. |
Fixes Install page floating $ by replacing certain code tags with var. Fixes Home page floating $ by replacing certain code tags with var.
@marc0der I'm really sorry about all the hassle. I went ahead with the |
No worries, I'll take another look over the weekend when I have some time. |
Looks like there are some conflicts with the most recent changes, I'll get them sorted tomorrow morning. |
Hi @davidbruce, As you probably realise, I had to revert #57 because it broke our site. To add to this, #58 continues adding more stuff that is not strictly related to the one thing you set out to do, which was placing focus on Getting started. Here is what I suggest: I love all your ideas about the following changes you wanted to bring about:
I would hate for us to miss out on your great ideas, so I would like to ask if you could consider raising three small PRs for these individual points. Having them separate makes it easier for us to review these changes and causes fewer problems with merging. Also, could I please ask you not to merge from master into your feature branch but rather to rebase against master before raising each PR? This results in a cleaner timeline in Git and makes things easier to maintain. Finally, because of the state of these branches, I suggest just doing these bits over and letting us review them one by one. Hope you don't mind and that you're up for this! 😄 |
Yeah I can split these into some PRs next week for ya. Sorry I let this fall by the wayside. |
@marc0der Here is a fix for floating
$
on certain inline text from #57. I created a new class calledmono-highlight
and replaced the inlinecode
tags withmono-highlight span
tagsLet me know if there are any other issues.