-
Notifications
You must be signed in to change notification settings - Fork 40
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
README.rst: Add Build and Development instructions #153
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, refer the issue in the commit message as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey,
Did you try following these steps and are these working for you?
Yea i followed these steps and they were working perfectly fine for me, Also i have send these instructions to one of my friends to cross verify it that whether he is facing any issues during installation of coala-html on Ubuntu or not, but he didn’t faced any issues 🙂 |
c87bb33
to
8a05f53
Compare
8a05f53
to
7e2ebcd
Compare
@frextrite @shashank-b Please review the latest commit |
@frextrite reply to comment |
f68f813
to
d65bf6b
Compare
@frextrite @shashank-b can you guys help me out in solving this issue as it is related to this issue only ? |
d65bf6b
to
a03b3fa
Compare
4055e4a
to
e66a039
Compare
@li-boxuan @frextrite @shashank-b Kindly review latest commits, tq |
e66a039
to
5ed25ad
Compare
README.rst
Outdated
|
||
:: | ||
|
||
$ sudo chown -R $USER:$GROUP ~/.config/configstore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if my review repeats things already in old reviews,...
This step is a non-standard step -- there is no reason why this directory should have strange permissions.
The problem is possibly caused by sudo npm install -g bower
, but I am not sure about that.
Anyways ... you added it here for a reason ... i'd love to know why, so we can look at how to avoid it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, when we try to run coala-html --dir ./coala_test_results
it install the bower components that are required to build up a static website and it is a package used by AngularJS, that is possible only if it has write access to configstore
and, to install bower
using npm it needs write access that's why i used sudo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using asdf solves those sudo problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, will try out them and if everything goes well, will use asdf.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jayvdb asdf has solved the problem of sudo npm install -g bower
but wasn't able to solve the problem sudo chown -R $USER:$GROUP ~/.config/configstore
It still need permissons to access the files in the configstore
folder.
npm suggests a ownership command sudo chown -R $USER:$(id -gn $USER) ~/.config/configstore
, we can change the above command to this one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KVGarg , if you never run 'sudo' in your install steps, the chown
is not necessary.
redo your steps without running 'sudo'. and show me errors that you can not fix. chown
is not the solution - it is a workaround recommended by npm because they do not recommend asdf
, and so their workaround is not acceptable to us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jayvdb I'm wondering that earlier it wasn't getting installed due to write access to configstore
but now it is getting installed easily without even using sudo
. Removing this instruction from guide.
README.rst
Outdated
:: | ||
|
||
$ sudo apt-get install curl python-software-properties | ||
$ curl -sL https://deb.nodesource.com/setup_11.x | sudo bash - |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesnt ubuntu have node? Above a "Node.js PPA" is mentioned, but this is not a PPA.
We should give a PPA install method.
Also for generic Linux, and even Windows, I am now recommending that we use @asdf-vm in our instructions, as it allows installing of node and python and anything else we need. (not sure about bower / lets check that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section is meant for Linux users, sorry my bad.
I will change to Linux
in both the headings where it is written for Ubuntu/Linux users
.
PPA install method
Should I change above heading to this one ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesnt ubuntu have node?
Need to run sudo apt install nodejs
to install nodejs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jayvdb And, should I add installation instructions according to @asdf-vm
in current instructions or a provide a link somewhere in instruction either at end or at the beginning of instructions ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A link to official upstream pages is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added link for installing nodejs via asdf-vm. Will it work? or should I add offical nodejs website link for installing it ?
5ed25ad
to
c58beff
Compare
This comment has been minimized.
This comment has been minimized.
c58beff
to
c310892
Compare
c310892
to
f8d84ef
Compare
f8d84ef
to
0092680
Compare
0092680
to
ac3d514
Compare
ac3d514
to
4e05c4f
Compare
The commit adds instructions for running coala-html on a local system.
Closes #135