Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(42)

Issue 97113: Rename genxml to bundle, and copy in required jars. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 11 months ago by do_not_use
Modified:
5 months, 2 weeks ago
Reviewers:
woodie
Base URL:
http://appengine-jruby.googlecode.com/svn/trunk/appengine-tools/
Visibility:
Public.

Patch Set 1 #

Patch Set 2 : Merge in dev_appserver. #

Total comments: 15

Patch Set 3 : Fix quotes #

Patch Set 4 : Fix typo #

Unified diffs Side-by-side diffs Delta from patch set Stats (+197 lines, -114 lines) Patch
M Rakefile View 1 chunk +1 line, -1 line 0 comments Download
M bin/dev_appserver.rb View 1 chunk +1 line, -1 line 0 comments Download
M lib/appengine-tools/appcfg.rb View 1 3 2 chunks +3 lines, -3 lines 0 comments Download
A lib/appengine-tools/bundler.rb View 1 2 1 chunk +186 lines, -0 lines 0 comments Download
M lib/appengine-tools/dev_appserver.rb View 2 1 chunk +6 lines, -5 lines 0 comments Download
D lib/appengine-tools/diligence.rb View 1 chunk +0 lines, -40 lines 0 comments Download
D lib/appengine-tools/genxml.rb View 1 1 chunk +0 lines, -64 lines 0 comments Download

Messages

Total messages: 6
do_not_use
12 years, 11 months ago (2009-07-24 17:57:05 UTC) #1
woodie
Do I need to get my changes in, to avoid a merge?
12 years, 11 months ago (2009-07-24 20:32:18 UTC) #2
do_not_use
Ok, I've merged in dev_appserver.rb
12 years, 11 months ago (2009-07-24 22:56:56 UTC) #3
woodie
this is great! http://codereview.appspot.com/97113/diff/1005/7 File lib/appengine-tools/appcfg.rb (right): http://codereview.appspot.com/97113/diff/1005/7#newcode77 Line 77: if command && ! NO_XML_COMMANDS.include?(command) ...
12 years, 11 months ago (2009-07-25 00:49:17 UTC) #4
do_not_use
12 years, 11 months ago (2009-07-25 01:05:06 UTC) #5
do_not_use
12 years, 11 months ago (2009-07-25 01:05:14 UTC) #6
http://codereview.appspot.com/97113/diff/1005/7
File lib/appengine-tools/appcfg.rb (right):

http://codereview.appspot.com/97113/diff/1005/7#newcode77
Line 77: if command && ! NO_XML_COMMANDS.include?(command)
What do you think would be useful? As long as the check's run quickly I don't
want to be too verbose.

On 2009/07/25 00:49:17, woodie wrote:
> we could also put some friendly feedback here
> like he have in the dev_appserver

http://codereview.appspot.com/97113/diff/1005/11
File lib/appengine-tools/bundler.rb (right):

http://codereview.appspot.com/97113/diff/1005/11#newcode105
Line 105: # TODO auto generate a config.ru
On 2009/07/25 00:49:17, woodie wrote:
> that sounds fantastic, but when/where does that happen?

below, instead of just printing an error message. I've moved the TODO to the
correct place.

http://codereview.appspot.com/97113/diff/1005/11#newcode108
Line 108: puts 'Error: you either need a #{app.config_ru} or '
On 2009/07/25 00:49:17, woodie wrote:
> these need to be double quotes

Done.

http://codereview.appspot.com/97113/diff/1005/11#newcode109
Line 109: puts '      #{app.aeweb_xml}.'
On 2009/07/25 00:49:17, woodie wrote:
> ...double quotes

Done.

http://codereview.appspot.com/97113/diff/1005/11#newcode113
Line 113: puts 'Error: you need to create #{app.config_ru}.'
On 2009/07/25 00:49:17, woodie wrote:
> ..double quotes

Done.

http://codereview.appspot.com/97113/diff/1005/11#newcode150
Line 150: # TODO if there's more than 1 we need to check the api version.
On 2009/07/25 00:49:17, woodie wrote:
> Having this part of the SDK makes this difficult.
> It would be nice to have a gem for this as well,
> and the appengin-spis depend on, or just included.

The sdk treats this jar specially. We can't modify it if we want things to work
right.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b