GDG-Ukraine / gdg.org.ua

GDG Ukraine website, event registration forms.
http://gdg.org.ua/
MIT License
6 stars 24 forks source link

[#89|fix] Fix setting up environment #90

Closed cleac closed 8 years ago

cleac commented 8 years ago

Fix #89

@anxolerd, could you checkout this?

webknjaz commented 8 years ago

Don't ever add sudo to scripts!11 That's bullshit! And don't force user installing stuff globally.

webknjaz commented 8 years ago

I thing this PR is totally broken

webknjaz commented 8 years ago

*think

cleac commented 8 years ago

@webknjaz ok. But you typed first npm install -g bower. What's the point of installing bower globally but not installing it globally...

Stop, I have an idea, what's else wrong with that script. The virtualenv is not opening while running make dev-deps. I'll look through it more and write a bit later

cleac commented 8 years ago

@webknjaz @anxolerd I figured out, what's wrong with that. The problem is in implemented building concept.

As you know, any of Makefile commands are called one-by-one by sh -c "__current_command__". So, when are you calling

if test -d ./$(PENV); then . ./$(PENV)/bin/activate; fi

works only until the current sh -c ... finishes. So the whole concept this Makefile is wrong Tested this on make 4.0 and 4.1.

webknjaz commented 8 years ago

1) bower needs to be installed globally. But depending on setup it may not require superuser privileges (e.g. one uses nvm or node is installed in userspace in some other way)

webknjaz commented 8 years ago

2) Yeah, I know about some of the design issues and haven't neither solved those, nor cleaned them. You may try doing this and we'll discuss your suggestions

cleac commented 8 years ago

I see now the point about bower, that's my mistake. But it's a bit not obvious that build system should use it, as for me... What you think about saying about nvm and virtualenv in README.md in **How to run it *** section?

About building environment script: I'm currently working on implementing setting up using .sh language and pull it ASAP

anxolerd commented 8 years ago

As for bower, I'd rather install it locally, and call it using executable in node_modules/bin/bower. As for sudo -- I totally agree with @webknjaz.

And I'm not good at bash scripting unfortunately. It would be better such changes will be reviewed by @webknjaz in the future.

cleac commented 8 years ago

@webknjaz could you checkout what I did here? I know, that is not much good solution of problem, but it works :smiley:

webknjaz commented 8 years ago

I don't like overall idea of replacing make with sh. Still, you may want to call a shell script from make targets, but removal of the Makefile doesn't look convenient. And I don't like this hack of making script executable, git should be able to keep this flag. Also, executables should be place into bin/

webknjaz commented 8 years ago

Bower itself usually asks to be available in global installation

cleac commented 8 years ago

The make is useless in this case :disappointed:, but sh script implements whole functionality with no problems (instead of a big file which implements that). Do you have any other ideas to implement this without sh and from clear environment without virtualenv set up? 'Cause I don't :cry:

cleac commented 8 years ago

I agree that bower likes to be installed globally, but it's not compatible with python's virtualenv. Another option is to require nvm to be installed for automatic environment set up. But that's another damn thing that has to be installed system-wide... Maybe we just should require the bower to be installed system-wide and that's all the solution?

webknjaz commented 8 years ago

nvm is not systemwide. It's installed in userspace by default. Anyway, we may require bower as a prerequisite. I've upgraded make workflow in master. Now it should work for you better. It's been WIP, thus being buggy. It shouldn't bother you anymore at this point.

cleac commented 8 years ago

@webknjaz: Erm... Why did you do such a strange fix - isn't that uglier than just simple shell script? But okay, there is no idea in discussing it more... Close #89 and #90 - they have no longer sense.