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

Changed name of arrayOfStringsWithTagPaths to tagPathStrings #4

Merged
merged 1 commit into from
Sep 13, 2022

Conversation

TeaBizzy
Copy link
Contributor

For issue #3. I'm proposing the variable name be changed to tagPathStrings. Feedback is welcome on the naming as I'm not sure if it will be suitable for everyone else.

Also updated the comments to reflect the change, and fixed a small inconsistency with the comment on line 38.

Note: I did get the code to run, and got the following output. I think it has to do with how my directories are setup:

Script started.
local_obj.json will be translated into other language version local_obj.json.
./exe.sh: line 37: /usr/bin/expect: No such file or directory
Script done.

@jacobkim9881
Copy link
Owner

Variable name arrayOfStringsWithTagPaths has type of the variable, type of value and what is values in but has a bit long name to read. tagPathStrings has shorter names easy to read.
Additional updates for comments and consoles are appreciated. Also good for updating one on line 38.
The log after executing is successful one. Latterly json output from master branch should be equal to the one from variableRename branch. But robots can do such works.
Thanks for pull request and congratulations for the first pull request on the repository. This pull request is accepted.

@jacobkim9881 jacobkim9881 merged commit 118fbf2 into jacobkim9881:master Sep 13, 2022
@TeaBizzy
Copy link
Contributor Author

Thank you @jacobkim9881. Your explanation of the issue and where to look to solve it made it a pleasure to contribute. I will keep an eye out, and contribute where I can in the future.

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.

2 participants