Skip to content

Conversation

@Prashant937
Copy link

@Prashant937 Prashant937 commented Jan 24, 2022

added tw-install.rc file
#83

added tw-install.rc file
Copy link
Contributor

@a-t-0 a-t-0 left a comment

Choose a reason for hiding this comment

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

Thanks! Nice that you also applied capitalisation for the Global variables! In full transparency, I clicked "approve" out of happiness to include your contribution. However, I reviewed your work in more detail, and found 3 minor changes that could improve the quality of the code base, hence I added a second review to allow me to "request changes". (I did not know how to change my "approved these changes" to "request changes" in the first review I gave on this pull request.) My apologies for the inconsistent response.

TASKWIKI_DIR=~/.task/wiki
TASKNOTES_DIR=~/.task/notes
TASKTHEME_DIR=~/.task/theme
BIN_DIR=~/.local/bin
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you perhaps be able to explain what the bin dir is for?
*Possibly as a comment, such that others coming into this repository also know what it is for?

Copy link
Author

Choose a reason for hiding this comment

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

This is just a sample code, you can let me know what to put in place of these

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for your response, I have looked up the source of the sample code, and assume this is that source: #83 and asked @linuxcaffe if he would be so kind to provide an elaboration on the two commented entries.

To answer your suggestion, I think the current list is a nice start (assuming a meaningful interpretation is given for the two commented entries in question:

BIN_DIR=~/.local/bin

and

WEB_DIR=~/var/www/something

And I would like to extend that list with:

LOG_LOCATION=src/logs/
TASKSERVER_TAR_NAME=taskd-1.1.0
TW_USERNAME=First
TW_ORGANISATION=Public
PORT=53589
TASKDDATA=/var/taskd

It would be nice if this list, which comes from the hardcoded_variables.txt could be removed from there, and if the respective loading of those variables is changed from hardcoded_variables.txt to tw-install.rc.

TASKNOTES_DIR=~/.task/notes
TASKTHEME_DIR=~/.task/theme
BIN_DIR=~/.local/bin
WEB_DIR=~/var/www/something
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you perhaps be able to explain what the WEB_DIR is for?
*Possibly as a comment, such that others coming into this repository also know what it is for?

@@ -0,0 +1,17 @@
### tw-install.rc configuration file
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps include a brief description of its contents, as the Google Style Guide for Bash recommends: https://google.github.io/styleguide/shellguide.html#file-header

Copy link
Author

Choose a reason for hiding this comment

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

sure will look into it

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