[GitHub] [tinkerpop] Cole-Greer commented on pull request #1952: patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors

["Cole-Greer (via GitHub)" <gi...@apache.org>]

Cole-Greer commented on PR #1952:
URL: https://github.com/apache/tinkerpop/pull/1952#issuecomment-1414457774

   @iAmmarTahir Thanks for catching this error and submitting this fix. I would like to add that this would be further improved if a default value of 'NotAvailable' was provided as in ``` const cpuArch = process?.arch?.replace(' ', '_') || 'NotAvailable';```
   
   Also I would like to ask if you could target this PR to 3.5-dev or 3.6-dev. Our master branch contains the code which will eventually make up the next major release (3.7.0). To have your changes brought into upcoming minor releases (3.5.6, 3.6.3) the changes will need to be in the corresponding dev branches. You should always target the earliest branch you wish to support as changes will be merged forward (eg. if your PR is merged into 3.5-dev, we will also merge it forward into 3.6-dev and master).
   
   One wrinkle is that the 3.5.x branch is using node v10 which does not support the `?.` operator. Therefore some less elegant null checks will be needed for that branch. If you are willing to submit a node v10 compatible fix to 3.5-dev that would be fantastic. Otherwise I would ask that you add the default value of 'NotAvailable' and target this PR to 3.6-dev and I can look into a fix for the 3.5.x branch.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org