Account Options

  1. Sign in
The old Google Groups will be going away soon, but your browser is incompatible with the new version.
Google Groups Home
« Groups Home
codereview-list
Home
+ new post
About this group
Join this group
Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Options
There are currently too many topics in this group that display first. To make this topic appear first, remove this option from another topic.
There was an error processing your request. Please try again.
flag
   13 messages - Collapse all  -  Translate all to Translated (View all originals)
The group you are posting to is a Usenet group. Messages posted to this group will make your email address visible to anyone on the Internet.
Your reply message has not been sent.
Your post was successful
 
From:
To:
Cc:
Followup To:
Add Cc | Add Followup-to | Edit Subject
Subject:
Validation:
For verification purposes please type the characters you see in the picture below or the numbers you hear by clicking the accessibility icon. Listen and type the numbers you hear
 
techto...@gmail.com  
View profile  
 More options Feb 13, 4:59 am
From: techto...@gmail.com
Date: Mon, 13 Feb 2012 09:59:46 +0000
Local: Mon, Feb 13 2012 4:59 am
Subject: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author
Reviewers: Andi Albrecht,

Description:
Make `make` require only one param to run and add 'serve' alias

Please review this at http://codereview.appspot.com/5651086/

Affected files:
   M Makefile

Index: Makefile
===================================================================
--- a/Makefile
+++ b/Makefile
@@ -1,15 +1,17 @@
  # Makefile to simplify some common AppEngine actions.
  # Use 'make help' for a list of commands.

-DEV_APPSERVER?= dev_appserver.py
+SDK_PATH ?= .
+
+DEV_APPSERVER?= $(SDK_PATH)/dev_appserver.py
  DEV_APPSERVER_FLAGS?=

-APPCFG?= appcfg.py
+APPCFG?= $(SDK_PATH)/appcfg.py
  APPCFG_FLAGS?=

  PYTHON?= python2.5
  COVERAGE?= coverage
-SDK_PATH?=
+

  default: help

@@ -17,6 +19,8 @@
        @echo "Available commands:"
        @sed -n '/^[a-zA-Z0-9_.]*:/s/:.*//p' <Makefile | sort

+run: serve
+
  serve: update_revision
        @echo "---[Starting SDK AppEngine Server]---"
        $(DEV_APPSERVER) $(DEV_APPSERVER_FLAGS) .


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
albrecht.a...@googlemail.com  
View profile  
 More options Feb 13, 5:07 am
From: albrecht.a...@googlemail.com
Date: Mon, 13 Feb 2012 10:07:01 +0000
Local: Mon, Feb 13 2012 5:07 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author

http://codereview.appspot.com/5651086/diff/1/Makefile
File Makefile (right):

http://codereview.appspot.com/5651086/diff/1/Makefile#newcode6
Makefile:6: DEV_APPSERVER?= $(SDK_PATH)/dev_appserver.py
This breaks setups where dev_appserver.py (and appcfg.py) is somewhere
in $PATH.

http://codereview.appspot.com/5651086/diff/1/Makefile#newcode22
Makefile:22: run: serve
Do we really need this extra alias?

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
techto...@gmail.com  
View profile  
 More options Feb 13, 5:12 am
From: techto...@gmail.com
Date: Mon, 13 Feb 2012 10:12:56 +0000
Local: Mon, Feb 13 2012 5:12 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author

http://codereview.appspot.com/5651086/diff/1/Makefile
File Makefile (right):

http://codereview.appspot.com/5651086/diff/1/Makefile#newcode6
Makefile:6: DEV_APPSERVER?= $(SDK_PATH)/dev_appserver.py
On 2012/02/13 10:07:01, Andi Albrecht wrote:

> This breaks setups where dev_appserver.py (and appcfg.py) is somewhere

in $PATH.

If I remove '.' from the SDK_PATH, the DEV_APPSERVER will be
'/dev_appserver.py'. Do you know a way to output '/' only if SDK_PATH is
set in Makefile?

http://codereview.appspot.com/5651086/diff/1/Makefile#newcode22
Makefile:22: run: serve
On 2012/02/13 10:07:01, Andi Albrecht wrote:

> Do we really need this extra alias?

I'd rename to 'run', because the task semantically tied to this command
is to make Rietveld running, not 'served' or 'serving'. At least that is
what I thought of when adding instructions on OpenHatch page.

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
techto...@gmail.com  
View profile  
 More options Feb 13, 5:26 am
From: techto...@gmail.com
Date: Mon, 13 Feb 2012 10:26:33 +0000
Local: Mon, Feb 13 2012 5:26 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author

 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
techto...@gmail.com  
View profile  
 More options Feb 13, 5:32 am
From: techto...@gmail.com
Date: Mon, 13 Feb 2012 10:32:09 +0000
Local: Mon, Feb 13 2012 5:32 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author
See the PatchSet no.2

http://codereview.appspot.com/5651086/diff/1/Makefile
File Makefile (right):

http://codereview.appspot.com/5651086/diff/1/Makefile#newcode6
Makefile:6: DEV_APPSERVER?= $(SDK_PATH)/dev_appserver.py
On 2012/02/13 10:12:56, techtonik wrote:

> On 2012/02/13 10:07:01, Andi Albrecht wrote:
> > This breaks setups where dev_appserver.py (and appcfg.py) is
somewhere in
> $PATH.
> If I remove '.' from the SDK_PATH, the DEV_APPSERVER will be
> '/dev_appserver.py'. Do you know a way to output '/' only if SDK_PATH
is set in
> Makefile?

Done.

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
albrecht.a...@googlemail.com  
View profile  
 More options Feb 13, 6:21 am
From: albrecht.a...@googlemail.com
Date: Mon, 13 Feb 2012 11:21:02 +0000
Local: Mon, Feb 13 2012 6:21 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author
LGTM - but I'm not sure if this works for all make implementations.

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Andi Albrecht  
View profile  
 More options Feb 13, 10:55 am
From: Andi Albrecht <albrecht.a...@googlemail.com>
Date: Mon, 13 Feb 2012 16:55:23 +0100
Local: Mon, Feb 13 2012 10:55 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author

On Mon, Feb 13, 2012 at 4:50 PM,  <mar...@chromium.org> wrote:
> On 2012/02/13 11:21:02, Andi Albrecht wrote:

>> LGTM - but I'm not sure if this works for all make implementations.

> ?

> I don' mind, lgtm.
> IMHO I'd prefer it to just look at ../google_appengine by default but
> that's me.

me too :)


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
techto...@gmail.com  
View profile  
 More options Feb 13, 1:57 pm
From: techto...@gmail.com
Date: Mon, 13 Feb 2012 18:57:43 +0000
Local: Mon, Feb 13 2012 1:57 pm
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author
I prefer to look for ../google_appengine by default too, but can't
immediately see [1] the way to provide compatibility for PATH setups
except rewriting Makefile with Python.

Anyway, committed the current fix. Thanks for review. =)

1.
http://sunsite.ualberta.ca/Documentation/Gnu/make-3.79/html_chapter/m...

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
techto...@gmail.com  
View profile  
 More options Feb 14, 1:09 am
From: techto...@gmail.com
Date: Tue, 14 Feb 2012 06:09:43 +0000
Local: Tues, Feb 14 2012 1:09 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author
On 2012/02/13 19:00:26, M-A wrote:

> On 2012/02/13 18:57:42, techtonik wrote:
> > I prefer to look for ../google_appengine by default too, but can't
immediately
> > see [1] the way to provide compatibility for PATH setups except
rewriting
> > Makefile with Python.
> In general, adding the AppEngine SDK in PATH doesn't make sense for
most people.
> And if all rietveld devs prefer to have SDK_PATH prepopulated, it's
probably a
> good idea to just do it.

So I'm going to change the default value to ../google_appengine then.
Andi? =)

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
albrecht.a...@googlemail.com  
View profile  
 More options Feb 15, 3:17 pm
From: albrecht.a...@googlemail.com
Date: Wed, 15 Feb 2012 20:17:03 +0000
Local: Wed, Feb 15 2012 3:17 pm
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author
On 2012/02/14 06:09:42, techtonik wrote:

> On 2012/02/13 19:00:26, M-A wrote:
> > On 2012/02/13 18:57:42, techtonik wrote:
> > > I prefer to look for ../google_appengine by default too, but can't
> immediately
> > > see [1] the way to provide compatibility for PATH setups except
rewriting
> > > Makefile with Python.

> > In general, adding the AppEngine SDK in PATH doesn't make sense for
most
> people.
> > And if all rietveld devs prefer to have SDK_PATH prepopulated, it's
probably a
> > good idea to just do it.
> So I'm going to change the default value to ../google_appengine then.

Andi? =)

I don't have a special opinion, "../google_appengine" is just where the
SDK lives in my setup. But in general I'm fine with how it is now.

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
techto...@gmail.com  
View profile  
 More options Feb 16, 2:29 am
From: techto...@gmail.com
Date: Thu, 16 Feb 2012 07:29:01 +0000
Local: Thurs, Feb 16 2012 2:29 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author

 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
techto...@gmail.com  
View profile  
 More options Feb 16, 2:31 am
From: techto...@gmail.com
Date: Thu, 16 Feb 2012 07:31:33 +0000
Local: Thurs, Feb 16 2012 2:31 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author
Adding comment manuall, because -m upload.py option didn't work.
Reopening as I've learned some more Makefile spells. Please, check. The
only leftover is the sane error message when SDK is not found.

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
techto...@gmail.com  
View profile  
 More options Feb 18, 3:00 am
From: techto...@gmail.com
Date: Sat, 18 Feb 2012 08:00:08 +0000
Local: Sat, Feb 18 2012 3:00 am
Subject: Re: Make `make` require only one param to run and add 'serve' alias (issue 5651086)
Print | Individual message | Show original | Report this message | Find messages by this author
Reopening the stale issue is a bad way to communicate. Closing.

http://codereview.appspot.com/5651086/


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
End of messages
« Back to Discussions « Newer topic     Older topic »

Create a group - Google Groups - Google Home - Terms of Service - Privacy Policy
©2012 Google