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

First attempt to markdownify the readme #6718

Closed
wants to merge 14 commits into from

Conversation

auge8472
Copy link
Contributor

As @TrueBrain stated in issue #6698 the readme should be adapted to Markdown. I did this with generating a header hierarchy and a bit of reformatting (lists, paragraphs, etc.). IMHO this is not the whole work.

  • The title of the document „Readme“ could be changed i.e. to „OpenTTD readme“ or something like that.
  • There are several file and path names, shell commands and such things. Should these items maybe marked as inlinecode?
  • A few lists are formatted with spaces between text parts to format them like a table. I formatted these blocks differently to show the possibilities. In „3.0 Supported platforms“ I used a code block to keep the format. In „4.2 OpenTTD directories“, section of directories for types and extensions, I used a list, what led to an more or less unformatted layout (the spaces are present in the source code but get not rendered by the browsers because of the HTML-rules). Especially the mentioned list could be changed to a table because of it's three colums (purpose/name, directory, notice).
  • The text lines have a limited length to make the readme.txtmore readable. When switching to Markdown, this limitation is gone. Is it useful to keep this old format?

Please let me know, what to improve. I will implement, what is needed. To remove the readme.txt would be the last step.

Copy link
Member

@TrueBrain TrueBrain left a comment

Choose a reason for hiding this comment

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

Wauw, this looks good :D Minor bits and pieces from my side, nothing drastic. I have no real comments :)

readme.md Outdated
## Meta

- Last updated: 2016-07-01
- Release version: 1.6.1
Copy link
Member

Choose a reason for hiding this comment

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

Possibly bump this :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aside the fact, that the informations are outdated (two years old), I tend to agree in general because it's a source of unnecessary maintenance effort.

readme.md Outdated
@@ -0,0 +1,766 @@
# Readme
Copy link
Member

Choose a reason for hiding this comment

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

Indeed, maybe just change this to "OpenTTD". But I agree with you, Readme alone is weird now :)

@LordAro
Copy link
Member

LordAro commented Apr 14, 2018

Should probably remove the old readme.txt as well :)
And I know it doesn't matter, but most projects all-caps the file - README.md.

@auge8472
Copy link
Contributor Author

Ok, I changed the documents title and removed the section with the documents date and the version number. Aside the cosmetic tasks only one problem remains (I think it is one). The inconsistent styling of the lists with (virtual) columns. Keeping the layout with the misuse of code markup (like in „3.0 Supported platforms“) is IMHO unsatisfying. Either the layout should be kept, then I tend to use tables or these lists should be lists (what they are from a semantic point of view (HTML-coding trained)), what would lead to a non-fixed layout.

Should probably remove the old readme.txt as well :)

I will remove it at the end. I only want to wait for the ok. It's a kind of clearing when the job is done. ;-)

@TrueBrain
Copy link
Member

Please do what you think makes the best README.md (and I agree, capitals indeed). As I really like what I see so far, and I am not really experienced with choices in markup :)

Another request, please rebase to latest master. It is the reason the checks are failing. We fixed this in the latest master, but .. you first need to rebase to there :D Tnx!

@LordAro
Copy link
Member

LordAro commented Apr 14, 2018

Since it uses markdown now, can you make the Table of Contents actually link to the correct sections? Not sure if overkill...

@auge8472
Copy link
Contributor Author

auge8472 commented Apr 14, 2018

Since it uses markdown now, can you make the Table of Contents actually link to the correct sections? Not sure if overkill...

It's possible and I did that somewhen. Was something with additional HTML-elements. Have to research, how I did it. :-)

edit: It's easier than I thought. Github generates ID as anchors from the headers. Simple copy'n'paste job.

@auge8472
Copy link
Contributor Author

auge8472 commented Apr 14, 2018

I have a problem after rebasing. The first three commits of the branch, already part of this pull request, seems to be doubled in the history of my repository (once before rebasing, a second time after it). Because of that Github does not accept a push to the online-branch. The only solution I know is to force the push (git push origin mdreadme -f). Would this break something in this pull request? How to handle it in a proper manner?

@LordAro
Copy link
Member

LordAro commented Apr 15, 2018 via email

@TrueBrain
Copy link
Member

Indeed, after a rebase you will always have to force push, as you changed history. What we commonly do, assuming you have a remote called 'upstream' pointing to https://github.com/OpenTTD/OpenTTD:

git fetch upstream
git rebase upstream/master
git push -f

You mention that you see commits double; this most often happens that if you do a 'pull' before a push. Git now merged your work twice; which is a bit silly :D Recovering from that is relative easy:

git rebase -i upstream/master

This will give you an editor which shows all your commits in your current branch; this is including the double commits. Now you just have to pick either one you want to keep. Save the file, close your editor, and you should now be able to do a git push -f.

Sorry for the trouble; normally not really needed, but we are still building up OpenTTD on GitHub :)

@auge8472
Copy link
Contributor Author

auge8472 commented Apr 15, 2018

Well, I rebased my fork again with the command @TrueBrain posted (git rebase -i upstream/master). The editor showed up and told me „nope“. No changes to rebase. The list of commits that you mentioned and that I also found in code examples, didn't show up. After that I pushed the code with -f to Github. The additional commits are now listed here. The two automatic checks still fails. :-(

@TrueBrain
Copy link
Member

Yeah, the checks fail because your commit messages don't comply to the commit style :) But we can fix that if we like the current result :)

@TrueBrain
Copy link
Member

Okay, everyone seems to like it! Now we just have to toy a bit with your git branch; because we would like it if we first have a commit where readme.txt is renamed to README.md, and then (in a single commit) your other changes happen.

Possibly the easiest way to do this, is to start clean. Below a suggestion how to do that, but feel free to pick what-ever works for you :)

  • Copy README.md to some location you can find it back.
  • Start a new branch (git checkout upstream/master -b mdreadme_2).
  • Move the readme.txt (git mv readme.txt README.md) and commit this.
  • Copy your backup of README.md over the current README.md, and commit this.
  • Push your new branch over your old: git push -f origin mdreadme_2:mdreadme

When committing, please mind the commit style: https://wiki.openttd.org/Commit_style#Commit_message

Thank you for this work! :)

@auge8472
Copy link
Contributor Author

Possibly the easiest way to do this, is to start clean.

I also thought about restarting the attempt. :-) Main work is done, so it's only a task of following the ruleset.

When committing, please mind the commit style

I've a few questions at this point. Not that I have to restart it again and again. Working under this strictness is new for me.

  • The matching keyword seems to be "Cleanup" because it's no fix and no real change and least a new feature. Does it fits best?
  • I mentioned the issue Developer docs updates for git / github move #6698 but the rewrite of the readme is there only one of the listed tasks. I' not sure if it's important to mention the issue in this case.

I tend to something like Cleanup #6698: … or Cleanup: ….

Is it recommended to open a new, clean pull request?

@TrueBrain
Copy link
Member

In this Pull Request is fine by me. A new is also fine, if that is easier :)

Commit messages .. always annoying .. :D I think best fit for this is simply Doc; and mentioning the issue .. what-ever feels best to you.

@auge8472
Copy link
Contributor Author

I opened a new, clean pull request. Thank you (especially @TrueBrain) for your help and patience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants