#374: ensure compile verbose pref is included on upload

[davegarthsimpson]

Motivation

Verbose output preferences reported as incorrect during upload actions.

Change description

Ensure compile "verbose" preference is considered when performing compile step prior to upload.

Other information

Upload "verbose" preference was being used for both compile and upload steps of the "upload operation".

Reviewer checklist

• PR addresses a single concern.
• The PR has no duplicates (please search among the Pull Requests before creating one)
• PR title and description are properly filled.
• Docs have been added / updated (for bug fixes / features)

[per1234]

This fixes #374 for me.

Thanks Dave!

[kittaakos]

Can we clean up the typings and the extra options argument for better APIs?

Here is the TypeScript way of redefining the verbose boolean type to { compile: boolean; upload: boolean } when uploading (using a programmer).

See the docs about Omit here.

[davegarthsimpson]

Can we clean up the typings and the extra options argument for better APIs?

Here is the TypeScript way of redefining the verbose boolean type to { compile: boolean; upload: boolean } when uploading (using a programmer).

See the docs about Omit here.

I don't mind changing it to this, I favoured the separation of compile and upload options to be more explicit, and maybe make things easier in case we need to pass additional compile specific options into the upload op in the future. What do you think @kittaakos?

[kittaakos]

I favoured the separation of compile and upload options to be more explicit

I do not want to introduce additional args in the service calls for a bug fix. First, the JS way is to provide one option, not multiple ones. Second, if the second arg is partial, a dev can specify additional options that are not used, only the verbose. What would be the expected behavior of this call with your API proposal:

coreService.upload({ board: { fqbn: 'a' }, verbose: true }, { board: { fqbn: 'b' }, verbose: false });

[davegarthsimpson]

I favoured the separation of compile and upload options to be more explicit

I do not want to introduce additional args in the service calls for a bug fix. First, the JS way is to provide one option, not multiple ones. Second, if the second arg is partial, a dev can specify additional options that are not used, only the verbose. What would be the expected behavior of this call with your API proposal:

coreService.upload({ board: { fqbn: 'a' }, verbose: true }, { board: { fqbn: 'b' }, verbose: false });

I appreciate that making a service call, usually you'd only expect one options object, but my thinking was: the doUpload call doesn't do just one thing, it also calls another service compile, so maybe there is value in being explicit about what options are used for each step.

No problem though, if the priority is to maintain the APIs' style + if you see no value in separating it out let's go for your suggested approach.

[davegarthsimpson]

when uploading (using a programmer).

@kittaakos what do you mean here by "using a programmer"? better verbose typings also works when usingProgrammer is false no?

[kittaakos]

I verified the build, and it worked as expected. 👍

@fstasi, could you please help with the review? Thank you!

[fstasi]

lgtm