docs(stark-all): improve Stark documentation #429

Mallikki · 2018-06-11T10:14:16Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/NationalBankBelgium/stark/blob/master/CONTRIBUTING.md#-commit-message-guidelines
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[x] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Previously, there was not a lot of documentation on the project. Compodoc stated that only 20% of the project was properly documented.

What is the new behavior?

The documentation covers now 94% of the project.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

coveralls · 2018-06-11T10:35:05Z

Coverage remained the same at 94.247% when pulling 14cb90f on Mallikki:feature/documentation into 3bd3eba on NationalBankBelgium:master.

coveralls · 2018-06-11T10:35:05Z

Coverage decreased (-0.01%) to 94.246% when pulling cc6d0e0 on Mallikki:feature/documentation into 0ced55c on NationalBankBelgium:master.

christophercr · 2018-06-11T12:38:40Z

packages/stark-build/config/json-server.common.js

@@ -154,7 +154,7 @@ function initializeServer(server, backends) {
 * Default Stark request transformations: from Stark API REST specifics into json-server API specifics
 * @param req {*} The current request
 * @param res {*} The current response
- * @param next {Function} Function to call next middleware (should always be called at the end of the current middleware)
+ * @param next: Function to call next middleware (should always be called at the end of the current middleware)


For JavaScript files I think we should leave the "{xyz}" types in params and returns, and in fact they are needed. The only place where we should remove them is from Typescript files ;)

christophercr · 2018-06-11T12:39:31Z

packages/stark-core/src/common/routes/routes.ts

 export const starkLoginStateName: string = "starkAppInit.starkLogin";
+/**
+ *	URL of the login state of the application


You can fix the formatting of this comment with Prettier ;)

christophercr · 2018-06-11T12:41:47Z

packages/stark-core/src/common/translations/translations.ts

@@ -1,4 +1,7 @@
 import { TranslateService } from "@ngx-translate/core";
+/**
+ *  _cloneDeep function, used to clone native and primitive types


There is no need to comment this kind of functions (all the ones from Lodash), we can simply skip them by just putting @ignore

christophercr · 2018-06-11T12:44:18Z

packages/stark-core/src/configuration/entities/application/app-config.entity.intf.ts

@@ -131,26 +136,26 @@ export interface StarkApplicationConfig {

 	/**
 	 * Get a back-end by name
-	 * @param name - Name of the back-end object to get
+	 * @param name :  Name of the back-end object to get


I suggest we use a hyphen (-) instead of a colon (:) before the description of the parameter, so we align to the JSDoc standard says ;) http://usejsdoc.org/tags-param.html

christophercr · 2018-06-11T12:47:16Z

packages/stark-core/src/configuration/entities/application/app-config.entity.ts

@@ -169,10 +171,20 @@ export class StarkApplicationConfigImpl implements StarkApplicationConfig {
 		return this.backends;
 	}

+	/**
+	 * If keepAlive is used in the application, the value of keepAliveDisabled is set to false


Perhaps this could describe it better:

"Check whether the keepAlive option in the StarkApplicationConfig is disabled"

and the '@returns' would be almost the same

christophercr · 2018-06-11T14:08:20Z

packages/stark-core/src/util/validation-class.util.ts

@@ -1,3 +1,6 @@
+/**
+ * Class used to validate that other classes are not undefined and/or not


Perhaps this can be like;

"Class containing utils to be used for class validation decorators of the 'class-validator' library"

christophercr · 2018-06-11T14:11:51Z

packages/stark-core/src/validation/validator.intf.ts

 export interface StarkValidator extends Validator {
+	/**
+	 * Function that allows the developer to set a minimum and/or maximum of selected items.


"Validates that the size of the given array is between the minimum and maximum limits defined" ;)

christophercr · 2018-06-11T14:27:02Z

packages/stark-core/src/validation/validators/is-bic/is-bic.validator.fn.ts

 export const starkIsBICValidatorName: string = "starkIsBIC";

 /**
+ * @ignore


Maybe we can move or copy this block to the place where this function is documented, cause this info seems to be important to undertand how this validation works

christophercr · 2018-06-11T14:27:17Z

packages/stark-core/src/validation/validators/is-isin/is-isin.validator.fn.ts

 export const starkIsISINValidatorName: string = "starkIsISIN";

 /**
+ * @ignore


Maybe we can move or copy this block to the place where this function is documented, cause this info seems to be important to undertand how this validation works

christophercr · 2018-06-11T14:29:13Z

starter/src/app/app.module.ts

 	const metadata: any = require("../stark-app-metadata.json");

 	return Deserialize(metadata, StarkApplicationMetadataImpl);
 }

 // TODO: where to put this factory function?
+/**
+ *	This method can be used to create and retrieve mock data


formatting...

christophercr · 2018-06-18T14:01:26Z

tslint.json

+      }
+    ],
+    "jsdoc-format": [true, "check-multiline-start"],
+    "no-redundant-jsdoc": true


I think these 3 new rules should be also added to these packages:

stark-core

stark-ui

starter

This is becasue the "npm run lint:all" script that is executed in Travis will lint all those packages separately ;)

christophercr · 2018-06-18T14:02:28Z

starter/src/app/translation.config.ts

 export function initializeTranslation(translateService: TranslateService): void {
 	translateService.addLangs(["en", "fr", "nl"]);
 	translateService.setDefaultLang("en");
 	translateService.use("nl");

+	/**
+	 * The English StarkLocal, composed of the "en" language code


typo: "StarkLocal" => "StarkLocale"

christophercr · 2018-06-18T14:02:34Z

starter/src/app/translation.config.ts

 	const english: StarkLocale = { languageCode: "en", translations: translationsEn };
+	/**
+	 * The French StarkLocal, composed of the "fr" language code


typo: "StarkLocal" => "StarkLocale"

christophercr · 2018-06-18T14:02:41Z

starter/src/app/translation.config.ts

 	const french: StarkLocale = { languageCode: "fr", translations: translationsFr };
+	/**
+	 * The Dutch StarkLocal, composed of the "nl" language code


typo: "StarkLocal" => "StarkLocale"

christophercr · 2018-06-18T14:06:28Z

showcase/src/app/app.component.spec.ts

@@ -1,3 +1,4 @@
+/* tslint:disable */


Let's avoid disabling tslint completely because then all rules will be disabled for this file.
So we should just disable those rules that are not needed/relevant in this case.
For example: /* tslint:disable:rule1 rule2 ruleN */

christophercr · 2018-06-18T14:13:03Z

packages/stark-core/src/modules/http/builder/http-request-builder.spec.ts

@@ -1566,7 +1565,7 @@ describe("Builder: StarkHttpRequestBuilder", () => {
 				undefinedField: null // due to Serialize => with undefined returns null
 				// emptyField: undefined   // empty values are omitted
 			});
-			/* tslint:enable */
+			/*tslint:disable:completed-docs*/


I think you mean this: /* tslint:enable */ which will enable all rules again

christophercr · 2018-06-18T14:13:23Z

packages/stark-core/src/modules/http/builder/http-abstract-base-request-builder.ts

@@ -1,7 +1,10 @@
+/* tslint:disable completed-docs*/


The correct format is: /* tslint:disable:completed-docs */ otherwise all rules are disabled ;)

christophercr · 2018-06-18T14:13:41Z

packages/stark-core/src/configuration/entities/application/app-config.entity.ts

@@ -1,11 +1,14 @@
+/* tslint:disable completed-docs*/


The correct format is: /* tslint:disable:completed-docs */otherwise all rules are disabled ;)

christophercr · 2018-06-18T14:14:02Z

packages/stark-core/src/common/error/error.ts

@@ -1,8 +1,11 @@
+/* tslint:disable completed-docs*/


The correct format is: /* tslint:disable:completed-docs */ otherwise all rules are disabled ;)

christophercr · 2018-06-18T14:15:04Z

packages/stark-core/src/modules/http/builder/http-abstract-fetch-resource-request-builder.ts

@@ -1,10 +1,13 @@
+/* tslint:disable completed-docs*/


The correct format is: /* tslint:disable:completed-docs */ otherwise all rules are disabled ;)

christophercr · 2018-06-18T14:21:30Z

@Mallikki Just some small changes to do regarding the disabling of tslint. You can see here the different ways to disable tslint depending on what you need:
https://palantir.github.io/tslint/usage/rule-flags/

ISSUES CLOSED: NationalBankBelgium#424, NationalBankBelgium#389

Mallikki force-pushed the feature/documentation branch from cc6d0e0 to 3fa9ec7 Compare June 11, 2018 10:39

Mallikki requested a review from christophercr June 11, 2018 10:40

Mallikki force-pushed the feature/documentation branch from 3fa9ec7 to 9be0321 Compare June 11, 2018 13:35

Mallikki mentioned this pull request Jun 11, 2018

doc: improve api docs #389

Closed

5 tasks

Mallikki force-pushed the feature/documentation branch from e68a744 to bbd096f Compare June 11, 2018 14:19

Mallikki mentioned this pull request Jun 11, 2018

build: add documentation coverage checks #424

Closed

christophercr reviewed Jun 11, 2018

View reviewed changes

Mallikki force-pushed the feature/documentation branch 2 times, most recently from 8323f8c to b94ee68 Compare June 12, 2018 15:20

dsebastien added this to the 10.0.0-alpha.3 milestone Jun 12, 2018

Mallikki force-pushed the feature/documentation branch 2 times, most recently from ac5e4b2 to 7bd4c37 Compare June 18, 2018 08:00

Mallikki changed the title ~~docs(core): improved documentation of stark-core~~ docs(core): improved documentation Stark Jun 18, 2018

Mallikki requested review from dsebastien and SuperITMan June 18, 2018 09:32

Mallikki force-pushed the feature/documentation branch from 7bd4c37 to 2f1fda5 Compare June 18, 2018 11:22

christophercr suggested changes Jun 18, 2018

View reviewed changes

Mallikki force-pushed the feature/documentation branch from 2f1fda5 to 19bbe48 Compare June 19, 2018 12:54

feat(docs): added TSlint rules and adapted documentation for the project

14cb90f

ISSUES CLOSED: NationalBankBelgium#424, NationalBankBelgium#389

Mallikki force-pushed the feature/documentation branch from 19bbe48 to 14cb90f Compare June 20, 2018 15:22

christophercr approved these changes Jun 21, 2018

View reviewed changes

christophercr changed the title ~~docs(core): improved documentation Stark~~ docs(core): improve Stark documentation Jun 21, 2018

christophercr changed the title ~~docs(core): improve Stark documentation~~ docs(stark-all): improve Stark documentation Jun 21, 2018

christophercr merged commit d93d64b into NationalBankBelgium:master Jun 21, 2018

Mallikki deleted the feature/documentation branch October 16, 2018 13:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(stark-all): improve Stark documentation #429

docs(stark-all): improve Stark documentation #429

Mallikki commented Jun 11, 2018 •

edited

Loading

coveralls commented Jun 11, 2018 •

edited

Loading

coveralls commented Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 11, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr Jun 18, 2018

christophercr commented Jun 18, 2018

		@@ -1,3 +1,6 @@
		/**
		* Class used to validate that other classes are not undefined and/or not

docs(stark-all): improve Stark documentation #429

docs(stark-all): improve Stark documentation #429

Conversation

Mallikki commented Jun 11, 2018 • edited Loading

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

coveralls commented Jun 11, 2018 • edited Loading

coveralls commented Jun 11, 2018

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christophercr commented Jun 18, 2018

Mallikki commented Jun 11, 2018 •

edited

Loading

coveralls commented Jun 11, 2018 •

edited

Loading