Skip to content

Greater dir naming flexibility. #245

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.

Sign up for GitHub

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

Merged
jkarni merged 1 commit into from
Jun 26, 2013
Merged

Greater dir naming flexibility. #245

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions scripts/create-dev-env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,23 @@ clone_repos() {

set_base_default() { # if PROJECT_HOME not set
# 2 possibilities: this is from cloned repo, or not
# this script is in "./scripts" if a git clone
this_repo=$(cd "${BASH_SOURCE%/*}/.." && pwd)
if [[ "${this_repo##*/}" = "edx-platform" && -d "$this_repo/.git" ]]; then
# set BASE one-up from this_repo;
echo "${this_repo%/*}"

# See if remote's url is named edx-platform (this works for forks too, but
# not if the name was changed).

@yarko yarko Jun 22, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

DO NOT IMPLEMENT (discussion only):
Could head this off by following the idiom as for $PROJECT_HOME set PROJECT_NAME to 'edx-platform' if not set (e.g. after line 120).

It would not be a bad habit to get out of hardwiring constants like this anyway (.virtualenv comes to mind).
The downside (for a PROJECT_NAME) is that would be 21 more lines to change, for dubious benefit (there is clear purpose for PROJECT_HOME).

@jkarni jkarni Jun 24, 2013

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

For rvm (and probably also virtualenv), I'm not sure it's worth not hardcoding the name. People can still make and switch to other ones, and I can't imagine that they have a -different- virtualenv that has something else entirely but is named edx-platform.

On the other hand, I do think it might be a good idea to have something as simple as PROJECT_NAME=edx-platform along with all the other constant defs, since we do use it in a couple of different places.

@yarko yarko Jun 24, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(continuing discussion only) - re: hardcoding: first: taking out literal values tends to immune your code from changes over time (e.g. .virtualenv was the default from virtualenvwrapper, but now is Envs).

The second (more significant) benefit: your setup code (at least snippets) will prove more portable, more easily adapted / modified to other sub-projects as they spring up for several reasons: [1] one (as opposed to many) thing to change (e.g. PROJECT_NAME), and [2] the entire idiom of get something from a variable encourages coding so as to "not make assumptions" [e.g. "what is the "name"?", "what are the parameters it needs?", etc.].

Of course, all that needs be weighed against obfuscation: clearer (simpler) is better. In the case of PROJECT_NAME, if I were starting from scratch, that would be a wash for me (could go either way); PROJECT_HOME is more concrete in motivation. Anyway, that's how I think about it. YMMV.

@jkarni jkarni Jun 26, 2013

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed. I don't think PROJECT_NAME is really worth it, but the (your) PROJECT_HOME change definitely was.

cd "$( dirname "${BASH_SOURCE[0]}" )"
this_repo=$(basename $(git ls-remote --get-url 2>/dev/null) 2>/dev/null) ||
echo -n ""

if [[ "x$this_repo" = "xedx-platform.git" ]]; then
# We are in the edx repo and already have git installed. Let git do the
# work of finding base dir:
echo "$(dirname $(git rev-parse --show-toplevel))"
else
echo "$HOME/edx_all"
fi
}




### START

PROG=${0##*/}
Expand Down