Improve developer docs by bajtos · Pull Request #1015 · loopbackio/loopback-next

bajtos · 2018-02-19T13:31:33Z

integrate MONOREPO and DEVELOPING guides
add "new package checklist"
sort entries in MONOREPO.md
add content from the wiki page Contributing

This is a follow-up for #977 and #1005 and requires #1013 to be landed first.

Checklist

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
Related API Documentation was updated
Affected artifact templates in packages/cli were updated
Affected example projects in packages/example-* were updated

bajtos · 2018-02-19T13:35:59Z

Preview links:

raymondfeng · 2018-02-19T18:16:15Z

MONOREPO.md

+The [loopback-next](https://github.com/strongloop/loopback-next) repository uses [lerna](https://lernajs.io/) to manage multiple packages for LoopBack 4.

-## Packages
+<!-- PLEASE KEEP THE TABLE ROWS SORTED ALPHABETICALLY BY PACKAGE NAME-->


Hmm, I intentionally sorted the packages by the nature of the modules such as common utilities, core, extensions, examples.

The problem with "order by nature of modules" is that it's subjective. There's definitely a case to be made for either ordering but I will say that as a "third party", I think it's always easier to reference things that are alphabetically ordered.

If you are going to group by functionality, you should create separate tables and name the groupings.

raymondfeng · 2018-02-19T18:22:48Z

@bajtos Great start. A few comments:

The README should have obvious links to other files (developing/monorepo/contributing).
Not sure why DEVELOPING.md is under docs while the rest are not.
We probably should have a section in DEVELOPING.md to cover how to add documentation for new features.

jannyHou · 2018-02-20T16:09:59Z

@bajtos #1013 landed 🚢 you can continue yours, thanks!

richardpringle

Only one suggested change

richardpringle · 2018-02-20T14:13:20Z

MONOREPO.md

+The [loopback-next](https://github.com/strongloop/loopback-next) repository uses [lerna](https://lernajs.io/) to manage multiple packages for LoopBack 4.

-## Packages
+<!-- PLEASE KEEP THE TABLE ROWS SORTED ALPHABETICALLY BY PACKAGE NAME-->


The problem with "order by nature of modules" is that it's subjective. There's definitely a case to be made for either ordering but I will say that as a "third party", I think it's always easier to reference things that are alphabetically ordered.

If you are going to group by functionality, you should create separate tables and name the groupings.

richardpringle · 2018-02-20T14:14:27Z

README.md

 ```

-> Make sure you set `"target": "es6"` in your compiler options in your
+> Make sure you set `"target": "es2017"` in your compiler options in your


richardpringle · 2018-02-20T14:14:53Z

README.md

- [Join the team](https://github.com/strongloop/loopback-next/issues/110)
+See [Contributing Guidelines](./CONTRIBUTING.md) for more details.
+
+You can join the team by posting a comment to [issue #110](https://github.com/strongloop/loopback-next/issues/110.).


richardpringle · 2018-02-20T14:16:29Z

docs/DEVELOPING.md

+We use two tools to keep our codebase healthy:
+
+ - [TSLint](https://palantir.github.io/tslint/) to statically analyse our source code and detect common problems.
+ - [Prettier](https://prettier.io/) to keep our code always formatted the same way, avoid style discussions in code reviews, and save everybody's time an energy.


and save everybody's time and energy

👍

richardpringle · 2018-02-20T14:22:48Z

docs/DEVELOPING.md


 Just as in the subject, use the imperative, present tense: "change" not "changed" nor "changes"a

+Paragraphs or bullet points are ok (must not exceed 100 characters per line). Typically a hyphen or asterisk is used for the bullet, followed by a single space, with blank lines in between, but conventions vary here.


but, conventions vary here.

I would remove this. While it may be true, I don't think it should be documented.

Will remove.

richardpringle · 2018-02-20T17:04:07Z

docs/DEVELOPING.md

+$ npm i -g commitizen
+```
+
+And to use it, simply call `git cz` instead of `git commit`. The tool will help you generate a commit message that follows the above guidelines.


Were the above guidelines directly from commitizen or is there some kind of config that sets things like the 100 char "limit".

Might be good to mention where "the above guidelines" come from.

Most of the guidelines are from https://conventionalcommits.org/, some tweaks are our own (100 char limit, list of scopes defined by monorepo packages). We do mention https://conventionalcommits.org/ at the beginning of Commit message guidelines section. I feel that's good enough.

dhmlau · 2018-02-24T03:20:21Z

docs/DEVELOPING.md

+ - Update [MONOREPO.md](../MONOREPO.md) - insert a new table row to describe the new package, please keep the rows sorted by package name.
+ - Update [docs/apidocs.html](./docs/apidocs.html) - add a link to API docs for this new package.
+ - Update [CODEOWNERS](./CODEOWNERS) - add a new entry listing the primary maintainers (owners) of the new package
+ - Ask somebody from the IBM team (e.g. [@bajtos](https://github.com/bajtos), [@raymondfeng](https://github.com/raymondfeng) or [@kjdelisle](https://github.com/kjdelisle)) to enlist the new package on http://apidocs.strongloop.com/


http://apidocs.strongloop.com/ -> http://apidocs.loopback.io/

dhmlau

Just one more change on the apidoc.loopback.io URL instead of the strongloop.com one.

bajtos · 2018-02-26T13:27:07Z

The README should have obvious links to other files (developing/monorepo/contributing).

Makes sense, will fix.

Not sure why DEVELOPING.md is under docs while the rest are not.

That's a good question, indeed.

I think it's better to keep CONTRIBUTING.md at project root to make it easier for people to find it, as CONTRIBUTING is one of those common files many projects have in project root. Since we are linking to this file from README, I don't mind to move it to docs. GitHub's feature showing a link to CONTRIBUTING file supports the docs location too, no problem there.
I did not move MONOREPO to docs because I though it was your intention to have it in project root. If you are ok with the move, then I am happy to move it to docs.

@raymondfeng Thoughts?

We probably should have a section in DEVELOPING.md to cover how to add documentation for new features.

I agree. Let's leave it out of scope of this pull request though.

[Raymond] I intentionally sorted the packages by the nature of the modules such as common utilities, core, extensions, examples.
[Richard] The problem with "order by nature of modules" is that it's subjective. There's definitely a case to be made for either ordering but I will say that as a "third party", I think it's always easier to reference things that are alphabetically ordered.
If you are going to group by functionality, you should create separate tables and name the groupings.

@raymondfeng what's your opinion here? May I keep alphabetical order, or are you asking me to preserve your sorting and split the table into multiple ones?

bajtos · 2018-02-26T13:28:24Z

I have rebased the patch on top of the latest master and addressed your review comments.

@raymondfeng @richardpringle LGTY now?

This reverts commit c8af3f6.

* docs: integrate MONOREPO and DEVELOPING guides * docs: add "new package checklist" * docs: sort entries in MONOREPO.md * docs: more README cleanup * docs: add content from the wiki page "Contributing"

bajtos added this to the February 2018 milestone Feb 19, 2018

bajtos self-assigned this Feb 19, 2018

bajtos requested review from b-admike, kjdelisle, raymondfeng, richardpringle and virkt25 February 19, 2018 13:31

bajtos added the needs-priority label Feb 19, 2018

This was referenced Feb 19, 2018

doc: add new package #1013

Merged

feat: @loopback/boot #858

Merged

Format the code using prettier #165

Closed

raymondfeng reviewed Feb 19, 2018

View reviewed changes

richardpringle suggested changes Feb 20, 2018

View reviewed changes

raymondfeng mentioned this pull request Feb 21, 2018

doc: Move contribution doc from wiki to repo #574

Closed

dhmlau reviewed Feb 24, 2018

View reviewed changes

dhmlau approved these changes Feb 24, 2018

View reviewed changes

bajtos force-pushed the docs/improvements branch from 5dd5eb0 to 2a6b090 Compare February 26, 2018 13:27

bajtos added developer-experience Issues affecting ease of use and overall experience of LB users Docs labels Feb 27, 2018

raymondfeng approved these changes Feb 27, 2018

View reviewed changes

bajtos added 5 commits February 27, 2018 18:02

docs: integrate MONOREPO and DEVELOPING guides

67022b8

docs: add "new package checklist"

99081ac

docs: sort entries in MONOREPO.md

7dd48f8

docs: more README cleanup

1067b58

docs: add content from the wiki page "Contributing"

916ede6

bajtos force-pushed the docs/improvements branch from 2a6b090 to 916ede6 Compare February 27, 2018 17:05

bajtos merged commit c8af3f6 into master Feb 27, 2018

bajtos removed the needs-priority label Feb 27, 2018

bajtos deleted the docs/improvements branch February 27, 2018 17:20

kjdelisle pushed a commit that referenced this pull request Feb 27, 2018

Revert "docs: Improve developer docs (#1015)"

15834cd

This reverts commit c8af3f6.


		Just as in the subject, use the imperative, present tense: "change" not "changed" nor "changes"a

		Paragraphs or bullet points are ok (must not exceed 100 characters per line). Typically a hyphen or asterisk is used for the bullet, followed by a single space, with blank lines in between, but conventions vary here.

Conversation

bajtos commented Feb 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

bajtos commented Feb 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raymondfeng commented Feb 19, 2018

Uh oh!

jannyHou commented Feb 20, 2018

Uh oh!

richardpringle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhmlau left a comment

Choose a reason for hiding this comment

Uh oh!

bajtos commented Feb 26, 2018

Uh oh!

bajtos commented Feb 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bajtos commented Feb 19, 2018 •

edited

Loading