Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

fix: add fieldMask for getDevice and listDevices #436

Merged
merged 4 commits into from
Feb 5, 2021
Merged

Conversation

gcseh
Copy link
Contributor

@gcseh gcseh commented Feb 4, 2021

Fixes #178

@gcseh gcseh requested a review from telpirion February 4, 2021 15:24
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 4, 2021
@product-auto-label product-auto-label bot added the api: cloudiot Issues related to the googleapis/nodejs-iot API. label Feb 4, 2021
@gcseh gcseh added the samples Issues that are directly related to samples. label Feb 4, 2021
@codecov
Copy link

codecov bot commented Feb 4, 2021

Codecov Report

Merging #436 (e6687a2) into master (556920f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #436   +/-   ##
=======================================
  Coverage   97.35%   97.35%           
=======================================
  Files           5        5           
  Lines        2683     2683           
  Branches      104      105    +1     
=======================================
  Hits         2612     2612           
  Misses         70       70           
  Partials        1        1           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 556920f...0501543. Read the comment docs.

@gcseh gcseh changed the title Add fieldMask for getDevice and listDevices fix: add fieldMask for getDevice and listDevices Feb 4, 2021
Copy link
Contributor

@telpirion telpirion left a comment

Choose a reason for hiding this comment

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

Two small comments, but I would definitely take care of them (to save yourself future linter errors).

const [response] = await iotClient.listDevices({parent: parentName});
// See full list of device fields: https://cloud.google.com/iot/docs/reference/cloudiot/rest/v1/projects.locations.registries.devices
// Warning! Use snake_case field names.
const fieldMask = { paths: [ 'id', 'name', 'num_id', 'credentials', 'last_heartbeat_time', 'last_event_time', 'last_state_time', 'last_config_ack_time', 'last_config_send_time', 'blocked', 'last_error_time', 'last_error_status', 'config', 'state', 'log_level', 'metadata', 'gateway_config' ] };
Copy link
Contributor

Choose a reason for hiding this comment

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

This is definitely a place where I would break this line up, for example:

const fieldMask = {
    paths: [
        'id',
        'name',
        ...
    ]
};

You can probably use prettier to automatically format it for you.

const [response] = await iotClient.getDevice({name: devicePath});
// See full list of device fields: https://cloud.google.com/iot/docs/reference/cloudiot/rest/v1/projects.locations.registries.devices
// Warning! Use snake_case field names.
const fieldMask = { paths: [ 'id', 'name', 'num_id', 'credentials', 'last_heartbeat_time', 'last_event_time', 'last_state_time', 'last_config_ack_time', 'last_config_send_time', 'blocked', 'last_error_time', 'last_error_status', 'config', 'state', 'log_level', 'metadata', 'gateway_config' ] };
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above, break into additional lines for readability.

@gcseh gcseh marked this pull request as ready for review February 5, 2021 12:01
@gcseh gcseh requested a review from a team as a code owner February 5, 2021 12:01
@gcseh gcseh merged commit a97ed10 into master Feb 5, 2021
@gcseh gcseh deleted the add-field-mask branch February 5, 2021 12:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api: cloudiot Issues related to the googleapis/nodejs-iot API. cla: yes This human has signed the Contributor License Agreement. samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve FieldMask documentation
2 participants