Fix #2271: Add documentation about minification flag and check for bad pattern before committed. #2298

makoscafee · 2016-07-26T06:38:39Z

added documentation about --prod_env flag, MINIFICATION and DEV_MODE variables and added the check for app.yaml for any change in MINIFICATION variable.
fix #2271.

makoscafee · 2016-07-26T06:44:54Z

Hi @gvishal PTAL.

gvishal · 2016-07-26T06:50:36Z

feconf.py

@@ -33,6 +33,10 @@
 IS_MINIFIED = os.environ.get('MINIFICATION') == 'True'

 # Whether we should serve the development or production experience.
+# This is environment specific variable(DEV_MODE) changes to true only


s/changes to true/changes to false

Here's an edited version: "DEV_MODE should only be changed to False in the production environment. To use minified resources in the development environment, change the MINIFICATION env variable in app.yaml to True."

@makoscafee -- I still don't understand what DEV_MODE is, though (I don't know what you mean by an "environment-specific variable"). Can you perhaps explain what it affects?

when server starts there are variable that are set based on the environment the server has been started from, and DEV_MODE is set to true only if the SERVER_SOFTWARE is development meaning is just run from the local computer.

Right, I get that from looking at the file. But what is the actual effect of DEV_MODE being set to True/False -- what does it affect?

ooh ok like adding numpy in development mode, clearing cookies, set the dev mode green label that appears only in dev_mode ... I think only those.

Ah, I see! OK, then add an additional comment:

When DEV_MODE is True, this indicates that we are not running in the production App Engine environment, which affects things like login/logout URLs, as well as third-party libraries that App Engine normally provides.

gvishal · 2016-07-26T06:55:19Z

@makoscafee Looks good! Left some minor comments. Thanks!

seanlip · 2016-07-26T06:57:15Z

scripts/pre_commit_linter.py

@@ -108,6 +108,12 @@
            'core/templates/dev/head/expressions/typeParserSpec.js')}
 }

+BAD_PATTERNS_YAML = {
+    'MINIFICATION: true': {
+        'message': 'Please set minification  to false(MINIFICATION: false)',


Please set the MINIFICATION env variable in app.yaml to False before committing.

codecov-io · 2016-07-26T07:11:08Z

Current coverage is 47.99% (diff: 100%)

Merging #2298 into develop will not change coverage

@@            develop      #2298   diff @@
==========================================
  Files           190        190          
  Lines         15921      15921          
  Methods        2733       2733          
  Messages          0          0          
  Branches       2696       2696          
==========================================
  Hits           7642       7642          
  Misses         8279       8279          
  Partials          0          0

Powered by Codecov. Last update fe5438e...bb7fb79

gvishal · 2016-07-26T07:21:45Z

scripts/start.sh

@@ -95,7 +95,7 @@ for arg in "$@"; do
  fi
 done

-# Set value of MINIFICATION variable in app.yaml
+# Updates value of MINIFICATION variable in app.yaml


s/Updates/Update

seanlip · 2016-07-26T07:43:36Z

app.yaml

@@ -110,5 +110,8 @@ libraries:
  version: '1.6.1'
 - name: webapp2
  version: '2.5.2'
+# This environmental variable is for serving minified resources
+# when set True. It is used to run server as prod environment


set True --> set to true

And let's not confuse "prod" with "minified". Maybe say: It allows minified resources to be used when running the server in the dev environment.

seanlip · 2016-07-26T07:45:24Z

A few more small comments, otherwise lgtm!

seanlip · 2016-07-26T08:01:30Z

LGTM. Thanks @makoscafee!

@gvishal, does it look good to you? If so, we can merge this.

gvishal · 2016-07-26T08:14:25Z

@makoscafee @seanlip LGTM!

…d pattern before committed. (#2298) * : * fix linting problems" * test the precommint minification check * fix lint error * remove unused variable * tryin to see if it changes * add pattern check for minification variable * fix linting error * addressed review comments * added more documentations for DEV_MODE variable * addresed sean review comments

…or bad pattern before committed. (oppia#2298) * : * fix linting problems" * test the precommint minification check * fix lint error * remove unused variable * tryin to see if it changes * add pattern check for minification variable * fix linting error * addressed review comments * added more documentations for DEV_MODE variable * addresed sean review comments

makoscafee added 8 commits July 26, 2016 00:47

:

bdc0e98

fix linting problems"

2903947

test the precommint minification check

d9c46dd

fix lint error

5088154

remove unused variable

162969f

tryin to see if it changes

304313a

add pattern check for minification variable

54e7528

fix linting error

6a9251d

makoscafee changed the title ~~Minification flag doc~~ Add documentation about minification flag and check for bad pattern before committed fix #2271 Jul 26, 2016

makoscafee changed the title ~~Add documentation about minification flag and check for bad pattern before committed fix #2271~~ Fix #2271: Add documentation about minification flag and check for bad pattern before committed. Jul 26, 2016

makoscafee added the PR: don't merge - IN REVIEW label Jul 26, 2016

makoscafee assigned gvishal Jul 26, 2016

gvishal reviewed Jul 26, 2016
View reviewed changes

seanlip reviewed Jul 26, 2016
View reviewed changes

addressed review comments

58f9679

gvishal reviewed Jul 26, 2016
View reviewed changes

added more documentations for DEV_MODE variable

8cb9130

seanlip reviewed Jul 26, 2016
View reviewed changes

addresed sean review comments

bb7fb79

seanlip added the PR: LGTM label Jul 26, 2016

seanlip removed the PR: don't merge - IN REVIEW label Jul 26, 2016

seanlip merged commit aa4e928 into oppia:develop Jul 26, 2016

makoscafee deleted the minification-flag-doc branch July 26, 2016 09:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #2271: Add documentation about minification flag and check for bad pattern before committed. #2298

Fix #2271: Add documentation about minification flag and check for bad pattern before committed. #2298

makoscafee commented Jul 26, 2016 •

edited

Loading

makoscafee commented Jul 26, 2016

gvishal Jul 26, 2016

seanlip Jul 26, 2016

makoscafee Jul 26, 2016

seanlip Jul 26, 2016

makoscafee Jul 26, 2016

seanlip Jul 26, 2016

gvishal commented Jul 26, 2016

seanlip Jul 26, 2016 •

edited

Loading

makoscafee Jul 26, 2016

codecov-io commented Jul 26, 2016 •

edited

Loading

gvishal Jul 26, 2016

seanlip Jul 26, 2016

seanlip commented Jul 26, 2016

seanlip commented Jul 26, 2016

gvishal commented Jul 26, 2016

Fix #2271: Add documentation about minification flag and check for bad pattern before committed. #2298

Fix #2271: Add documentation about minification flag and check for bad pattern before committed. #2298

Conversation

makoscafee commented Jul 26, 2016 • edited Loading

makoscafee commented Jul 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gvishal commented Jul 26, 2016

seanlip Jul 26, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jul 26, 2016 • edited Loading

Current coverage is 47.99% (diff: 100%)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanlip commented Jul 26, 2016

seanlip commented Jul 26, 2016

gvishal commented Jul 26, 2016

makoscafee commented Jul 26, 2016 •

edited

Loading

seanlip Jul 26, 2016 •

edited

Loading

codecov-io commented Jul 26, 2016 •

edited

Loading