Migrate DeveloperGuide.md from markdown to asciidoc #271 #408

limmlingg · 2017-05-11T08:59:51Z

Part of #271

mention-bot · 2017-05-11T08:59:53Z

@limmlingg, thanks for your PR! By analyzing the history of the files in this pull request, we identified @damithc, @pyokagan and @jia1 to be potential reviewers.

CanIHasReview-bot · 2017-05-11T09:01:17Z

v1

@limmlingg submitted v1 for review.

[v1 1/1] Migrate documentation from markdown to asciidoc

(📚 Archive)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/408/1/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you want to fetch this PR to.

Zhiyuan-Amos · 2017-05-11T23:34:02Z

docs/DeveloperGuide.adoc

+(For all use cases below, the *System* is the `AddressBook` and the
+*Actor* is the `user`, unless specified otherwise)
+
+Use case: Delete person


I think it's unnecessary to have this as a "bullet point" in the table of content, because this is the only sub-point :)

Zhiyuan-Amos · 2017-05-11T23:35:12Z

docs/DeveloperGuide.adoc

@@ -0,0 +1,510 @@
+= AddressBook Level 4 - Developer Guide:
+:toc:


Is it possible to mark up the Table of Contents? The surrounding text are all marked up, but TOC isn't :P

Also, perhaps the inter-line spacing between each item could be wider; I noticed that it's quite small compared to the numbered points in later segments (e.g 1.1 Prerequisites)

I don't think I can edit the attributes of the ToC to make the line spacing wider. I can remove the title Table of Contents though, since it's not in DeveloperGuide.md

Zhiyuan-Amos · 2017-05-11T23:36:51Z

docs/DeveloperGuide.adoc

+
+[NOTE]
+====
+Having any Java 8 version is not enough. +


Possible to centralise this text such that the top border space == bottom border space?

Also I'm unsure if Note is necessary. @yamgent

http://asciidoctor.org/docs/asciidoc-syntax-quick-reference/

Can take a look at the admonition block. It should help :)

Alternatively, take a look at literal as well.
Also, this segment too: Markdown-style blockquote with block content

Zhiyuan-Amos · 2017-05-11T23:39:53Z

docs/DeveloperGuide.adoc

+Having any Java 8 version is not enough. +
+This app will not work with earlier versions of Java 8.
+====
+.  *Eclipse* IDE


Numbering error, should be 2

Zhiyuan-Amos · 2017-05-11T23:46:17Z

docs/DeveloperGuide.adoc

+image::LogicClassDiagram.png[width="800"]
+_Figure 2.1.2 : Class Diagram of the Logic Component_
+
+==== Events-Driven nature of the design


No need to number it as 2.1.1. This is because there isn't a 2.1.2 :)

Zhiyuan-Amos · 2017-05-11T23:57:29Z

docs/DeveloperGuide.adoc

+.  Using Chrome, go to the
+link:UsingGithubPages.md#viewing-the-project-site[GitHub Pages version]
+of the documentation file. e.g. For UserGuide.md, the URL will be
+`https://<your-username-or-organization-name>.github.io/addressbook-level4/docs/UserGuide.html`.


this shouldn't be hyperlinked

Zhiyuan-Amos · 2017-05-11T23:58:04Z

docs/DeveloperGuide.adoc

+Book depends on the http://wiki.fasterxml.com/JacksonHome[Jackson
+library] for XML parsing. Managing these _dependencies_ can be automated
+using Gradle. For example, Gradle can download the dependencies
+automatically, which is better than these alternatives. a. Include those


a and b should have a line break

Zhiyuan-Amos · 2017-05-11T23:59:08Z

docs/DeveloperGuide.adoc

+3a. The given index is invalid
+
+____
+3a1. AddressBook shows an error message Use case resumes at step 2


Use case resumes at step 2 should have a line break

Zhiyuan-Amos · 2017-05-12T00:00:25Z

docs/DeveloperGuide.adoc

+
+*Using Gradle*:
+
+* See UsingGradle.md for how to run tests using Gradle.


missing hyperlink

Zhiyuan-Amos · 2017-05-12T00:01:03Z

docs/DeveloperGuide.adoc

+multiple code units as well as how the are connected together. e.g.
+`seedu.address.logic.LogicManagerTest`
+
+=== Headless GUI Testing


I'm not sure whether this is meant to be a segment i.e 4.1 @yamgent ?

It is fine to leave it as a segment, that was probably the intention anyway.

Zhiyuan-Amos · 2017-05-12T00:03:10Z

docs/UsingGithubPages.adoc

+`Theme chooser`. Pick a theme for the project site and click
+`Select theme` when done.
+.  You can now view the site at
+`https://<username-or-organization-name>.github.io/<repo-name>`. e.g.


shouldn't be hyperlinked

Zhiyuan-Amos · 2017-05-12T00:03:50Z

docs/UsingGithubPages.adoc

+== Viewing the Project Site
+
+The project site URL follows the format
+`https://<username-or-organization-name>.github.io/<repo-name>`, e.g.


shouldn't be hyperlinked

Zhiyuan-Amos · 2017-05-12T00:04:03Z

docs/UsingGithubPages.adoc

+
+For the other pages, the structure of the site follows the structure of
+the repository. For example, `docs/UserGuide.md` is published at
+`https://<username-or-organization-name>.github.io/addressbook-level4/docs/UserGuide.html`.


shouldn't be hyperlinked

Zhiyuan-Amos · 2017-05-12T00:04:13Z

docs/UsingGithubPages.adoc

+
+The project site URL follows the format
+`https://<username-or-organization-name>.github.io/<repo-name>`, e.g.
+`https://se-edu.github.io/addressbook-level4`. By default, the


shouldn't be hyperlinked

Zhiyuan-Amos · 2017-05-12T00:07:02Z

docs/UsingTravis.adoc

+someone push code to the repo:
+
+* Runs the `./gradlew clean headless allTests coverage coveralls -i`
+command (see UsingGradle.md for more details on what this command


UsingGradle.md missing hyperlink

yamgent

I am having some problems with DeveloperGuide.adoc, it is failing my asciidoctor build:

~/github/yamgent/addressbook-level4$ ./gradlew asciidoctor
:copyStylesheets
:asciidoctor
(ArgumentError) asciidoctor: FAILED: /home/wangleng/github/yamgent/addressbook-level4/docs/DeveloperGuide.adoc: Failed to load AsciiDoc document - invalid byte sequence in UTF-8
:asciidoctor FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':asciidoctor'.
> Error running Asciidoctor

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

BUILD FAILED

Total time: 3.772 secs

yamgent · 2017-05-12T00:41:48Z

docs/UsingGithubPages.adoc

+using source files in the `master` branch. Jekyll, a static site
+generator integrated with GitHub Pages, automatically renders Markdown
+files (in the `master` branch) to HTML, which are then deployed to the
+project site by GitHub Pages.


Since we are touching on the documentation now, could be a good opportunity to avoid hard wrapping in our documentation. In other words, this whole paragraph should be in a single line.

(as per oss-generic/process#83, to avoid unnecessary noise to the diff when we change this documentation in the future and the wrapping goes out of sync).

@damithc, should we do that?

yamgent · 2017-05-12T00:42:13Z

~~@limmlingg Will you also be incorporating the other documents into the PR as well? (e.g. The other files in docs/)~~

EDIT: As discussed, we will try one file first.

CanIHasReview-bot · 2017-05-12T03:48:02Z

v2

@limmlingg submitted v2 for review.

[v2 1/1] Migrate DeveloperGuide.md from markdown to asciidoc

(📚 Archive) (📈 Interdiff between v1 and v2)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/408/2/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you want to fetch this PR to.

yamgent

[v2]

yamgent · 2017-05-12T03:55:43Z

docs/DeveloperGuide.adoc

@@ -0,0 +1,393 @@
+= AddressBook Level 4 - Developer Guide:


There is a colon at the end of the title, is that intended?

yamgent · 2017-05-12T04:04:36Z

docs/DeveloperGuide.adoc

+=== Architecture
+
+image::Architecture.png[width="600"]
+_Figure 2.1.1 : Architecture Diagram_


Just curious, does asciidocotor support figure numbering?

It numbers the figures based on their order of appearance, not based on the header number. So the Architecture Diagram will be Figure 1 instead of Figure 2.1.1, and Component interactions for delete 1 command (part 1) will be Figure 3 instead of Figure 2.1.3a

Ok sure, then we will use our manual figure numbering then.

Zhiyuan-Amos · 2017-05-12T05:37:43Z

docs/DeveloperGuide.adoc

+:sectnums:
+:imagesDir: images
+
+By : `Team SE-EDU`      Since: `Jun 2016`      Licence: `MIT`


There's an additional space here before the colon

Zhiyuan-Amos · 2017-05-12T05:45:43Z

docs/DeveloperGuide.adoc

+[NOTE]
+====
+* If you are asked whether to 'keep' or 'overwrite' config files, choose to 'keep'.
+* Depending on your connection speed and server load, it can even take up to 30 minutes for the set up to finish (This is because Gradle downloads library files from servers during the project set up process)


missing a period at end of sentence

CanIHasReview-bot · 2017-05-12T06:41:38Z

v3

@limmlingg submitted v3 for review.

[v3 1/1] Migrate DeveloperGuide.md from markdown to asciidoc

(📚 Archive) (📈 Interdiff between v2 and v3)

Checkout this PR version locally

git fetch https://github.com/se-edu/addressbook-level4.git refs/pr/408/3/head:BRANCHNAME

where BRANCHNAME is the name of the local branch you want to fetch this PR to.

yamgent · 2017-05-16T02:59:19Z

@kychua For your review please. :)

eugenepeh

Only concern is the asciidoc lists uses extra space compare to markdown. Can it actually be fixed?

yamgent · 2017-05-19T05:49:34Z

@eugenepeh That is the way GitHub renders asciidoc, so can't do much with the extra spaces.

kychua

@limmlingg Sorry for the very late review, just a few things I noticed

kychua · 2017-05-28T16:47:43Z

docs/DeveloperGuide.adoc

+Having any Java 8 version is not enough. +
+This app will not work with earlier versions of Java 8.
+
+[start=2]


should use list continuation here, like in User Guide

Yup! Since this has been merged, I'll be fixing this in another PR. Thanks for pointing it out!

kychua · 2017-05-28T16:48:48Z

docs/DeveloperGuide.adoc

+a. Include those libraries in the repo (this bloats the repo size) +
+b. Require developers to download those libraries manually (this creates extra work for developers)
+
+== Appendix A : User Stories


Don't need to number appendices A, B, C etc manually, can just use

[appendix] == User Stories

instead of

== Appendix A : User Stories

so it'll be

1. Testing 2. Dev Ops Appendix A: User Stories

instead of

1. Testing 2. Dev Ops 3. Appendix A: User Stories

kychua · 2017-05-28T17:19:07Z

docs/DeveloperGuide.adoc

+* At app launch: Initializes the components in the correct sequence, and connects them up with each other.
+* At shut down: Shuts down the components and invokes cleanup method where necessary.
+
+link:#common-classes[*`Commons`*] represents a collection of classes used by multiple other components. Two of those classes play important roles at the architecture level.


The link doesn't seem to work in html. I think you'll have to add idseparator: '-' to build.gradle's asciidoctor attributes.

yamgent · 2017-05-29T00:04:43Z

Thanks for taking the time to review @kychua. :)

limmlingg · 2017-05-29T01:47:23Z

I'll be making these fixes in #452

CanIHasReview-bot added the s.ToReview label May 11, 2017

Zhiyuan-Amos suggested changes May 12, 2017

View reviewed changes

yamgent suggested changes May 12, 2017

View reviewed changes

yamgent reviewed May 12, 2017

View reviewed changes

yamgent added s.Ongoing and removed s.ToReview labels May 12, 2017

limmlingg changed the title ~~Migrate documentation from markdown to asciidoc #271~~ Migrate DeveloperGuide.md from markdown to asciidoc #271 May 12, 2017

limmlingg force-pushed the 271-migrate-documentation-markdown-asciidoc branch 2 times, most recently from 326df2b to c6fa7b9 Compare May 12, 2017 03:47

CanIHasReview-bot added s.ToReview and removed s.Ongoing labels May 12, 2017

CanIHasReview-bot requested review from yamgent and Zhiyuan-Amos May 12, 2017 03:48

yamgent reviewed May 12, 2017

View reviewed changes

Zhiyuan-Amos suggested changes May 12, 2017

View reviewed changes

Migrate DeveloperGuide.md from markdown to asciidoc

71c07c2

limmlingg force-pushed the 271-migrate-documentation-markdown-asciidoc branch from c6fa7b9 to 71c07c2 Compare May 12, 2017 06:40

CanIHasReview-bot requested review from yamgent and Zhiyuan-Amos May 12, 2017 06:41

Zhiyuan-Amos approved these changes May 12, 2017

View reviewed changes

yamgent approved these changes May 12, 2017

View reviewed changes

yamgent requested review from kychua and chao1995 May 12, 2017 07:19

yamgent requested review from damithc and removed request for chao1995 May 16, 2017 02:59

damithc approved these changes May 18, 2017

View reviewed changes

yamgent added s.ToMerge and removed s.ToReview labels May 18, 2017

eugenepeh reviewed May 18, 2017

View reviewed changes

yamgent merged commit 1af6587 into se-edu:master May 19, 2017

kychua reviewed May 28, 2017

View reviewed changes

limmlingg mentioned this pull request Jul 19, 2017

Migrate from Eclipse to IntelliJ #421 #427

Merged

		@@ -0,0 +1,510 @@
		= AddressBook Level 4 - Developer Guide:
		:toc:


		Using Gradle:

		* See UsingGradle.md for how to run tests using Gradle.

Migrate DeveloperGuide.md from markdown to asciidoc #271 #408

Migrate DeveloperGuide.md from markdown to asciidoc #271 #408

Conversation

limmlingg commented May 11, 2017 • edited Loading

mention-bot commented May 11, 2017

CanIHasReview-bot commented May 11, 2017

v1

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

limmlingg May 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamgent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamgent commented May 12, 2017 • edited Loading

CanIHasReview-bot commented May 12, 2017

v2

yamgent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CanIHasReview-bot commented May 12, 2017

v3

yamgent commented May 16, 2017

eugenepeh left a comment

Choose a reason for hiding this comment

yamgent commented May 19, 2017

kychua left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yamgent commented May 29, 2017

limmlingg commented May 29, 2017

limmlingg commented May 11, 2017 •

edited

Loading

limmlingg May 12, 2017 •

edited

Loading

yamgent commented May 12, 2017 •

edited

Loading