mirror of
https://github.com/balena-io/balena-cli.git
synced 2024-12-24 07:46:39 +00:00
8978221866
Change-type: patch
192 lines
10 KiB
Markdown
192 lines
10 KiB
Markdown
# Contributing
|
|
|
|
balenaCLI is an open source project and your contribution is welcome!
|
|
|
|
* Install the dependencies listed in the [NPM Installation
|
|
section](./INSTALL-ADVANCED.md#npm-installation) section of the installation instructions. Check
|
|
the section [Additional Dependencies](./INSTALL-ADVANCED.md#additional-dependencies) too.
|
|
* Clone the `balena-cli` repository (or a [forked
|
|
repo](https://docs.github.com/en/free-pro-team@latest/github/getting-started-with-github/fork-a-repo),
|
|
if you are not in the balena team), `cd` to it and run `npm install`.
|
|
* Build the CLI with `npm run build` or `npm test`, and execute it with `./bin/balena`
|
|
(on a Windows command prompt, you may need to run `node .\bin\balena`).
|
|
|
|
In order to ease development:
|
|
|
|
* `npm run build:fast` skips some of the build steps for interactive testing, or
|
|
* `npm run test:source` skips testing the standalone zip packages (which is rather slow)
|
|
* `./bin/balena-dev` uses `ts-node/register` to transpile on the fly.
|
|
|
|
Before opening a PR, test your changes with `npm test`. Keep compatibility in mind, as the CLI is
|
|
meant to run on Linux, macOS and Windows. balena CI will run test code on all three platforms, but
|
|
this will only help if you add some test cases for your new code!
|
|
|
|
## Semantic versioning, commit messages and the ChangeLog
|
|
|
|
The CLI version numbering adheres to [Semantic Versioning](http://semver.org/). The following
|
|
header/row is required in the body of a commit message, and will cause the CI build to fail if absent:
|
|
|
|
```
|
|
Change-type: patch|minor|major
|
|
```
|
|
|
|
Version numbers and commit messages are automatically added to the `CHANGELOG.md` file by the CI
|
|
build flow, after a pull request is merged. It should not be manually edited.
|
|
|
|
If `package.json` is updated for dependencies listed in the `repo.yml` file (like `balena-sdk`),
|
|
the commit message body should include a line in the following format:
|
|
```
|
|
Update balena-sdk from 12.0.0 to 12.1.0
|
|
```
|
|
|
|
This allows the CI to produce nested change logs (with expandable arrows), pulling in commit
|
|
messages from the upstream repositories. The following npm script can be used to automatically
|
|
produce a commit with a suitable commit message:
|
|
```
|
|
npm run update balena-sdk ^12.1.0
|
|
```
|
|
|
|
The script will create a new branch (only if `master` is currently checked out), run `npm update`
|
|
with the given target version and commit the `package.json` and `npm-shrinkwrap.json` files. The
|
|
script by default will set the `Change-type` to `patch` or `minor`, depending on the semver change
|
|
of the updated dependency. For a `major` change type, it can specified as an extra argument:
|
|
```
|
|
npm run update balena-sdk ^12.14.0 patch
|
|
npm run update balena-sdk ^13.0.0 major
|
|
```
|
|
|
|
## Editing documentation files (README, INSTALL, Reference website...)
|
|
|
|
The `doc/cli.markdown` file is automatically generated by running `npm run build:doc` (which also
|
|
runs as part of `npm run build`). That file is then pulled by scripts in the
|
|
[balena-io/docs](https://github.com/balena-io/docs/) GitHub repo for publishing at the [CLI
|
|
Documentation page](https://www.balena.io/docs/reference/cli/).
|
|
|
|
The content sources for the auto generation of `doc/cli.markdown` are:
|
|
|
|
* [Selected
|
|
sections](https://github.com/balena-io/balena-cli/blob/v12.23.0/automation/capitanodoc/capitanodoc.ts#L199-L204)
|
|
of the README file.
|
|
* The CLI's command documentation in source code (`lib/commands/` folder), for example:
|
|
* `lib/commands/push.ts`
|
|
* `lib/commands/env/add.ts`
|
|
|
|
The README file is manually edited, but subsections are automatically extracted for inclusion in
|
|
`doc/cli.markdown` by the `getCapitanoDoc()` function in
|
|
[`automation/capitanodoc/capitanodoc.ts`](https://github.com/balena-io/balena-cli/blob/master/automation/capitanodoc/capitanodoc.ts).
|
|
|
|
The `INSTALL*.md` and `TROUBLESHOOTING.md` files are also manually edited.
|
|
|
|
## Windows
|
|
|
|
The `npm run build:installer` script (which generates the `.exe` executable installer on Windows)
|
|
specifically requires [MSYS2](https://www.msys2.org/) to be installed. Other than that, the
|
|
standard Command Prompt or PowerShell can be used (though MSYS2 is still handy, as it provides
|
|
'git' and a number of common unix utilities). If changes are made to npm scripts in `package.json`,
|
|
check that they also run on a standard Windows Command Prompt.
|
|
|
|
## Updating the 'npm-shrinkwrap.json' file
|
|
|
|
The `npm-shrinkwrap.json` file is used to control package dependencies, as documented at
|
|
https://docs.npmjs.com/files/shrinkwrap.json.
|
|
|
|
Changes to `npm-shrinkwrap.json` can be automatically merged by git during operations like
|
|
`rebase`, `pull` and `cherry-pick`, but in some cases this results in suboptimal dependency
|
|
resolution (the `node_modules` folder may end up larger than necessary, with consequences to CLI
|
|
load time too). For this reason, the recommended way to update `npm-shrinkwrap.json` is to run
|
|
`npm install`, possibly alongside `npm dedupe` as well. The following commands can be used to
|
|
fix shrinkwrap issues and optimize the dependencies:
|
|
|
|
```sh
|
|
git checkout master -- npm-shrinkwrap.json
|
|
rm -rf node_modules
|
|
npm install # update npm-shrinkwrap.json to satisfy changes to package.json
|
|
npm dedupe # deduplicate dependencies from npm-shrinkwrap.json
|
|
npm install # re-add optional dependencies removed by dedupe
|
|
git add npm-shrinkwrap.json # add it for committing (solve merge errors)
|
|
```
|
|
|
|
Note that `npm dedupe` should always be followed by `npm install`, as shown above, even if
|
|
`npm install` had already been executed before `npm dedupe`.
|
|
|
|
Optionally, these steps may be automated by installing the
|
|
[npm-merge-driver](https://www.npmjs.com/package/npm-merge-driver):
|
|
|
|
```sh
|
|
npx npm-merge-driver install -g
|
|
```
|
|
|
|
## TypeScript and oclif
|
|
|
|
The CLI currently contains a mix of plain JavaScript and
|
|
[TypeScript](https://www.typescriptlang.org/) code. The goal is to have all code written in
|
|
Typescript, in order to take advantage of static typing and formal programming interfaces.
|
|
The migration towards Typescript is taking place gradually, as part of maintenance work or
|
|
the implementation of new features.
|
|
|
|
Of historical interest, the CLI was originally written in [CoffeeScript](https://coffeescript.org)
|
|
and used the [Capitano](https://github.com/balena-io/capitano) framework. All CoffeeScript code was
|
|
migrated to either Javascript or Typescript, and Capitano was replaced with oclif. A few file or
|
|
variable names still refer to this legacy, for example `automation/capitanodoc/capitanodoc.ts`.
|
|
|
|
## Programming style
|
|
|
|
`npm run build` also runs [balena-lint](https://www.npmjs.com/package/@balena/lint), which automatically
|
|
reformats the code. Beyond that, we have a preference for Javascript promises over callbacks, and for
|
|
`async/await` over `.then()`.
|
|
|
|
## Common gotchas
|
|
|
|
One thing that most CLI bugs have in common is the absence of test cases exercising the broken
|
|
code, so writing some test code is a great idea. Having said that, there are also some common
|
|
gotchas to bear in mind:
|
|
|
|
* Forward slashes ('/') _vs._ backslashes ('\') in file paths. The Node.js
|
|
[path.sep](https://nodejs.org/docs/latest-v12.x/api/path.html#path_path_sep) variable stores a
|
|
platform-specific path separator character: the backslash on Windows and the forward slash on
|
|
Linux and macOS. The
|
|
[path.join](https://nodejs.org/docs/latest-v12.x/api/path.html#path_path_join_paths) function
|
|
builds paths using such platform-specific path separator. However:
|
|
* Note that Windows (kernel, cmd.exe, PowerShell, many applications) accepts ***both*** forward
|
|
slashes and backslashes as path separators (including mixing them in a path string), so code
|
|
like `mypath.split(path.sep)` may fail on Windows if `mypath` contains forward slashes. The
|
|
[path.parse](https://nodejs.org/docs/latest-v12.x/api/path.html#path_path_parse_path) function
|
|
understands both forward slashes and backslashes on Windows, and the
|
|
[path.normalize](https://nodejs.org/docs/latest-v12.x/api/path.html#path_path_normalize_path)
|
|
function will _replace_ forward slashes with backslashes.
|
|
* In [tar](https://en.wikipedia.org/wiki/Tar_(computing)#File_format) streams sent to the Docker
|
|
daemon and to balenaCloud, the forward slash is the only acceptable path separator, regardless
|
|
of the OS where the CLI is running. Therefore, `path.sep` and `path.join` should never be used
|
|
when handling paths in tar streams! `path.posix.join` may be used instead of `path.join`.
|
|
|
|
* Avoid using the system shell to execute external commands, for example:
|
|
`child_process.exec('ssh "arg1" "arg2"');`
|
|
`child_process.spawn('ssh "arg1" "arg2"', { shell: true });`
|
|
Besides the usual security concerns of unsanitized strings, another problem is to get argument
|
|
escaping right because of the differences between the Windows 'cmd.exe' shell and the Unix
|
|
'/bin/sh'. For example, 'cmd.exe' doesn't recognize single quotes like '/bin/sh', and uses the
|
|
caret (^) instead of the backslash as the escape character. Bug territory! Most of the time,
|
|
it is possible to avoid relying on the shell altogether by providing a Javascript array of
|
|
arguments:
|
|
`spawn('ssh', ['arg1', 'arg2'], { shell: false});`
|
|
To allow for logging and debugging, the [which](https://www.npmjs.com/package/which) package may
|
|
be used to get the full path of a command before executing it, without relying on any shell:
|
|
`const fullPath = await which('ssh');`
|
|
`console.log(fullPath); # 'C:\WINDOWS\System32\OpenSSH\ssh.EXE'`
|
|
`spawn(fullPath, ['arg1', 'arg2'], { shell: false });`
|
|
|
|
* Avoid the `instanceof` operator when testing against classes/types from external packages
|
|
(including base classes), because `npm install` may result in multiple versions of the same
|
|
package being installed (to satisfy declared dependencies) and a false negative may result when
|
|
comparing an object instance from one package version with a class of another package version
|
|
(even if the implementations are identical in both packages). For example, once we fixed a bug
|
|
where the test:
|
|
`error instanceof BalenaApplicationNotFound`
|
|
changed from true to false because `npm install` added an additional copy of the `balena-errors`
|
|
package to satisfy a minor `balena-sdk` version update:
|
|
`$ find node_modules -name balena-errors`
|
|
`node_modules/balena-errors`
|
|
`node_modules/balena-sdk/node_modules/balena-errors`
|
|
In the case of subclasses of `TypedError`, a string comparison may be used instead:
|
|
`error.name === 'BalenaApplicationNotFound'`
|