Skip to content
This repository has been archived by the owner on Jul 31, 2019. It is now read-only.

adding separators to README.md #2474

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

adding separators to README.md #2474

wants to merge 7 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 9, 2017

this PR does basically aim at:

  • adding separators to README.md file;

the main motivation for that does basically lie within:

  • making file easier to be read;
  • making file simpler to be read;
  • making more direct to be read;
  • making file easier to be understood;
  • making file simpler to be understood;
  • making more direct to be understood;

it also aims at beautifying the file

**this PR does basically aim at:**
- *adding separators to README.md file*;

---

**the main motivation for that does basically lie within:**
- *making file easier to be read;*
- *making file simpler to be read;*
- *making more direct to be read;*
- *making file easier to be understood;*
- *making file simpler to be understood;*
- *making more direct to be understood;*

--- 

**it also aims at beautifying the file**
@ryanwarsaw
Copy link
Contributor

Heya, I had a quick question before I get into the PR, I've been noticing a lot of PRs across Mozilla repositories filing similar changes and using the same PR description template, and I'm curious as to what that template is and if it's being used in a class or workshop?

These changes will actually make it harder to read in it's current state, the dividers aren't consistent across all sections of header tags, and in some cases we're dividing sections that should really be kept together because they're all apart of the same section.

I'll leave it up to the rest of the crew to decide if these are changes we want to make, but we'd certainly need to make at least a couple changes if we decided to merge this.

@gideonthomas
Copy link
Contributor

@tmm88 I'm inclined to agree with @ryanwarsaw. Can you link to documentation that indicates the need for separators for accessibility purposes?

@ghost
Copy link
Author

ghost commented Sep 10, 2017

no, it's no being used in a class or workshop. that's just my drive for compulsive, obsession and flow, while i am contributing to open source (and I am being true and honest)

and I'm curious as to what that template is and if it's being used in a class or workshop?


I don't necessarily agree

These changes will actually make it harder to read in it's current state, the dividers aren't consistent across all sections of header tags, and in some cases we're dividing sections that should really be kept together because they're all apart of the same section.


I don't think we need to remove contributions. just optimizing them

I'll leave it up to the rest of the crew to decide if these are changes we want to make, but we'd certainly need to make at least a couple changes if we decided to merge this.


NO (because i don't have such a documentation, even if I would like). that's just my intuition, and feeling that tells me that this works fine. knowledge can be systematically empirical, and that's what I do, most of the times

Can you link to documentation that indicates the need for separators for accessibility purposes?

@flukeout
Copy link
Contributor

flukeout commented Sep 13, 2017

Hi @tmm88 - thanks for jumping in on this. Personally, I'm OK with the separators, but as @ryanwarsaw mentioned there are some other inconsistencies and improvements we could make as well, if you are up for it.

  • Let's only make the Thimble heading an h1
  • All the subsequent headings should be h2, including Localization and Getting ready to Publish etc.
    • Each h2 should have a divider above it
    • The Automated Installation and Manual Installation should be h3 and should not have dividers above them
  • In your PR Localization doesn't have a divider above it, but should
  • Remove the divider at the end of the Readme file
  • Please add an index that links to each of the headings to make this Readme easier to use
    • Let's make this a bulleted list
    • Add indented bullets for the installation type headings

Let me know your thoughts and thanks for your PR @tmm88

@ghost
Copy link
Author

ghost commented Sep 23, 2017

hey @flukeout i was out for a couple of days. i will jump right away into this as soon as i can do so

tmm88 added 2 commits September 27, 2017 22:51
**this PR aims at attempting to:**
- *fixing layout/markup hierarchy of the document*
- *fixing correlation between dividers and markup*
- *adding an index*
**this PR aims at attempting to:**
- *fixing layout/markup hierarchy of the document*
- *fixing correlation between dividers and markup*
- *adding an index*

---

it's the first time i am making an index, so i will need some help to tweak it up
@ghost
Copy link
Author

ghost commented Sep 27, 2017

finally:

  • got done within trying to review your suggestions
  • the index got a bit messt, so i would like to request help to fix it.
  • all the rest seems fine. please feel free to drop any feedback

@flukeout
Copy link
Contributor

flukeout commented Sep 28, 2017

Hey @tmm88 - great stuff. Here's an example of how to link the headings that you've got in the index at the top:

To link a heading, you have to replace the spaces with dashes and make everything lowercase.

  • Manually Installing the Parts becomes.. #manually-installing-the-parts

If you click on the little link icon next to any heading, you can see in the browser's address bar how that title needs to be changed for the link.

Thanks!

@ghost
Copy link
Author

ghost commented Sep 30, 2017

Greetings

I did basically go throughout:

  • reviewing all the suggestions.


Even though:

  • the whole thing doesn't run as it theoretically should


on the basis of this:

  • is the fact that some links are broken


the main reason for that is:

  • the fact that i can't figure out how to deal with curly brackets, etc., in indexes


used some \ but it didn't workout

@flukeout
Copy link
Contributor

flukeout commented Oct 2, 2017

Thanks for your continued work on this @tmm88 - In testing the readme on yoru branch, I noticed that the 1st, 2nd and 4th link in the index don't work. I didn't go through all of them yet, can you please check to make sure all of the links work? Thank you!

@ghost
Copy link
Author

ghost commented Oct 4, 2017

I already tried it myself. couldn't figure that out. but i think it's because of special characters

@gideonthomas
Copy link
Contributor

@tiagomoraismorgado88, yes it's because of the special characters. When fragment ids are created (for e.g. #automated-installation), all special characters including spaces are stripped out (spaces are replaced with -). The links that you use have characters like \ and ( in them. Remove those and the links should work.

@flukeout
Copy link
Contributor

flukeout commented Oct 4, 2017

Additionally, if you hover over the Link icon next to the heading, you can see the formatting of the link at the bottom of the browser you are using, as seen below...

image

@ghost
Copy link
Author

ghost commented Oct 4, 2017

done. everything working now

@gideonthomas
Copy link
Contributor

@tiagomoraismorgado88, the first link is still busted, you'll need to remove the / from the link

@ghost
Copy link
Author

ghost commented Oct 10, 2017

done

@gideonthomas
Copy link
Contributor

@flukeout pinging for re-review

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants