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

docs: add basic usage to README #980

Merged
merged 6 commits into from
Nov 26, 2023
Merged

docs: add basic usage to README #980

merged 6 commits into from
Nov 26, 2023

Conversation

TinsFox
Copy link
Contributor

@TinsFox TinsFox commented Nov 15, 2023

Update docs

Description

As a new user, I prefer to see how it was done in the beginning rather than upgrading the documentation. This can feel strange to new users

@ayushmanchhabra
Copy link
Collaborator

I recently moved all documentation from README to JSDoc comments so that it can be autogenerated as a website. I don't really see an issue with not having install instructions on the README if a link to a docs site is given.

Copy link
Contributor

@sysrage sysrage left a comment

Choose a reason for hiding this comment

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

I do agree basic usage instructions should be included in the README. The documentation can/should be used more as an API reference if/when basic usage isn't enough.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@ayushmanchhabra
Copy link
Collaborator

What do you mean by basic usage? I'm guess I'm too familiar with the options so everything to me seems like basic usage to me :P

@sysrage
Copy link
Contributor

sysrage commented Nov 15, 2023

What do you mean by basic usage? I'm guess I'm too familiar with the options so everything to me seems like basic usage to me :P

Essentially what's being added in this PR.

@TinsFox
Copy link
Contributor Author

TinsFox commented Nov 16, 2023

What do you mean by basic usage? I'm guess I'm too familiar with the options so everything to me seems like basic usage to me :P

It's how to use the library quickly to get him to do something that's expected. Just the most basic configuration is fine, probably the effect of a command like 'nwbuild --mode=build', if this command automatically fills in the other default parameters

@TinsFox
Copy link
Contributor Author

TinsFox commented Nov 16, 2023

I recently moved all documentation from README to JSDoc comments so that it can be autogenerated as a website. I don't really see an issue with not having install instructions on the README if a link to a docs site is given.

I see that, but I recommend removing this line

doc/*.md

because keeping the Markdown file allows us to add some extra instructions to it, rather than completely handing it over to the program

@ayushmanchhabra
Copy link
Collaborator

ayushmanchhabra commented Nov 17, 2023

I recently moved all documentation from README to JSDoc comments so that it can be autogenerated as a website. I don't really see an issue with not having install instructions on the README if a link to a docs site is given.

I see that, but I recommend removing this line

doc/*.md

because keeping the Markdown file allows us to add some extra instructions to it, rather than completely handing it over to the program

I understand what you mean. This is how I initially managed the docs. I'm trying to automate as many things as I can so that the maintainer(s) after me will have it easier. Right now docs gen is far from what I want it to be but working on making it better.

@ayushmanchhabra ayushmanchhabra changed the title docs: update docs docs: add basic usage to README Nov 17, 2023
@ayushmanchhabra
Copy link
Collaborator

@TinsFox I've made a couple of commit suggestions

@TinsFox
Copy link
Contributor Author

TinsFox commented Nov 26, 2023

@TinsFox I've made a couple of commit suggestions

I can't see your comment. Did you forget to submit it?

@ayushmanchhabra
Copy link
Collaborator

Ah I think you merged main into the branch which is why the commit suggestions got hidden

Copy link
Collaborator

@ayushmanchhabra ayushmanchhabra left a comment

Choose a reason for hiding this comment

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

I'm fine with the changes here. I have to figure out how to setup docs nicely which I'll do in subsequent PRs.

README.md Show resolved Hide resolved
With npm:

```shell
npm install nw-builder -D
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
npm install nw-builder -D
npm install -D nw-builder

With yarn:

```shell
yarn add nw-builder -D
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
yarn add nw-builder -D
yarn add -D nw-builder

With pnpm:

```shell
pnpm add nw-builder -D
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
pnpm add nw-builder -D
pnpm add -D nw-builder

Comment on lines +44 to +59
Here is two way to use nw-build to build your nwjs applications

### CLI

1. To get nwjs cache
```bash
nwbuild --mode=get
```
2. To run nwjs application
```bash
nwbuild --mode=run
```
3. To build nwjs application
```bash
nwbuild --mode=build
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Here is two way to use nw-build to build your nwjs applications
### CLI
1. To get nwjs cache
```bash
nwbuild --mode=get
```
2. To run nwjs application
```bash
nwbuild --mode=run
```
3. To build nwjs application
```bash
nwbuild --mode=build
```
Here are two ways to use nw-builder:
### CLI
1. To get and cache NW.js:
```shell
nwbuild --mode=get
```
2. To run your NW.js application
```shell
nwbuild --mode=run
```
3. To build your NW.js application
```shell
nwbuild --mode=build
```

2. Create a build script
```javascript
// scripts/build.js
const { nwbuild } = require("nw-builder");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
const { nwbuild } = require("nw-builder");
import { nwbuild } from "nw-builder";

Copy link
Collaborator

Choose a reason for hiding this comment

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

Direct CJS import does not work. You need to import it via the import("nwbuild") function call.

Comment on lines +77 to +83
version: "latest",
flavor: "normal",
platform: "linux",
arch: "x64",
outDir: "./build",
cache: false,
app: { ... },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
version: "latest",
flavor: "normal",
platform: "linux",
arch: "x64",
outDir: "./build",
cache: false,
app: { ... },

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we remove these options to match the CLI usage?

@ayushmanchhabra ayushmanchhabra merged commit 968eae9 into nwutils:main Nov 26, 2023
ayushmanchhabra added a commit that referenced this pull request Jan 22, 2024
Refs: #980

## Description of Changes

Move useful usage docs from JSDoc code comments to `README.md`.
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