[GitHub] [cassandra] josh-mckenzie commented on a diff in pull request #2163: CASSANDRA-18012 trunk: CircleCI: Remove -l/-m/-h flags and have two options: free or paid

["josh-mckenzie (via GitHub)" <gi...@apache.org>]

josh-mckenzie commented on code in PR #2163:
URL: https://github.com/apache/cassandra/pull/2163#discussion_r1110039156


##########
.circleci/readme.md:
##########
@@ -149,44 +154,37 @@ default medium config and Large containers.
 ## Updating the config
 For configuration changes meant to be permanent in the Apache repo you should never edit
 the `config.yml` file manually. Instead, you should edit the `config-2_1.yml` file and then

Review Comment:
   So to my earlier question, if we could name this something like `config-template.yml` it'd make the relationship between things a lot more innately clear I think.



##########
.circleci/generate.sh:
##########
@@ -30,13 +30,12 @@ die ()
 
 print_help()
 {
-  echo "Usage: $0 [-l|-m|-h|-f|-e]"
-  echo "   -a Generate the default config.yml using low resources and the three templates"
-  echo "      (config.yml.LOWRES, config.yml.MIDRES and config.yml.HIGHRES). Use this for"
-  echo "      permanent changes in config-2_1.yml that will be committed to the main repo."
-  echo "   -l Generate config.yml using low resources"
-  echo "   -m Generate config.yml using mid resources"
-  echo "   -h Generate config.yml using high resources"
+  echo "Usage: $0 [-f|-p|-a|-e|-i]"
+  echo "   -a Generate the default config.yml using resources compatible with CircleCI's free tier"

Review Comment:
   nit: referring to `config.yml` on -a, -f, and -p muddies it up and it's not immediately clear how the relationship works. Noodling on this a bit, maybe something like:
   ```
   -a Generate a new default config.yml to be committed after changes to the template <etc>
   -f Generate a config.yml for tests compatible with the CircleCI free tier resources
   -f Generate a config.yml for tests compatible with the CircleCI paid tier resources
   ```
   
   It's subtle, but forklifting the "hey, we're generating a new default config to be committed" up front as well as the slight massaging on -f and -p imply the relationship between these params and the workflow for the config in a way the phrasing here doesn't.
   
   Maybe. Take it or leave it. 😀 I know how it works and had to squint a bit and think hard to make sure I grokked the difference though.



##########
.circleci/readme.md:
##########
@@ -25,22 +25,27 @@ The file `config.yml` is the configuration file that is read by CircleCI. This f
 automatically generated by the `generate.sh` script from the `config-2_1.yml` file.
 
 The provided `config.yml` file uses low resources so users of the CircleCI free tier can

Review Comment:
   nit... ish?
   
   Maybe we could just phrase this as something like:
   ```
   The provided default `config.yaml` file targets a subset of tests that succeed on the free tier of CircleCI; this is a duplicate of the config.yml.FREE file for generating purposes. There is also an optional config.yml.PAID file that includes more test suites that require the higher resources available on premium CircleCI accounts, which we notably also use for validating new releases.
   ```



##########
.circleci/generate.sh:
##########
@@ -145,52 +141,38 @@ if $has_env_vars && $check_env_vars; then
   done
 fi
 
-if $lowres; then
-  ($all || $midres || $highres) && die "Cannot use option -l with options -a, -m or -h"
-  echo "Generating new config.yml file with low resources from config-2_1.yml"
-  circleci config process $BASEDIR/config-2_1.yml > $BASEDIR/config.yml.LOWRES.tmp
-  cat $BASEDIR/license.yml $BASEDIR/config.yml.LOWRES.tmp > $BASEDIR/config.yml
-  rm $BASEDIR/config.yml.LOWRES.tmp
-
-elif $midres; then
-  ($all || $lowres || $highres) && die "Cannot use option -m with options -a, -l or -h"
-  echo "Generating new config.yml file with middle resources from config-2_1.yml"
-  patch -o $BASEDIR/config-2_1.yml.MIDRES $BASEDIR/config-2_1.yml $BASEDIR/config-2_1.yml.mid_res.patch
-  circleci config process $BASEDIR/config-2_1.yml.MIDRES > $BASEDIR/config.yml.MIDRES.tmp
-  cat $BASEDIR/license.yml $BASEDIR/config.yml.MIDRES.tmp > $BASEDIR/config.yml
-  rm $BASEDIR/config-2_1.yml.MIDRES $BASEDIR/config.yml.MIDRES.tmp
+if $free; then
+  ($all || $paid) && die "Cannot use option -f with options -a or -p"
+  echo "Generating new config.yml file for free tier from config-2_1.yml"

Review Comment:
   For my own learning: why is this file named `config-2_1.yml`. 2.1 of what?



##########
.circleci/generate.sh:
##########
@@ -145,52 +141,38 @@ if $has_env_vars && $check_env_vars; then
   done
 fi
 
-if $lowres; then
-  ($all || $midres || $highres) && die "Cannot use option -l with options -a, -m or -h"
-  echo "Generating new config.yml file with low resources from config-2_1.yml"
-  circleci config process $BASEDIR/config-2_1.yml > $BASEDIR/config.yml.LOWRES.tmp
-  cat $BASEDIR/license.yml $BASEDIR/config.yml.LOWRES.tmp > $BASEDIR/config.yml
-  rm $BASEDIR/config.yml.LOWRES.tmp
-
-elif $midres; then
-  ($all || $lowres || $highres) && die "Cannot use option -m with options -a, -l or -h"
-  echo "Generating new config.yml file with middle resources from config-2_1.yml"
-  patch -o $BASEDIR/config-2_1.yml.MIDRES $BASEDIR/config-2_1.yml $BASEDIR/config-2_1.yml.mid_res.patch
-  circleci config process $BASEDIR/config-2_1.yml.MIDRES > $BASEDIR/config.yml.MIDRES.tmp
-  cat $BASEDIR/license.yml $BASEDIR/config.yml.MIDRES.tmp > $BASEDIR/config.yml
-  rm $BASEDIR/config-2_1.yml.MIDRES $BASEDIR/config.yml.MIDRES.tmp
+if $free; then
+  ($all || $paid) && die "Cannot use option -f with options -a or -p"
+  echo "Generating new config.yml file for free tier from config-2_1.yml"
+  circleci config process $BASEDIR/config-2_1.yml > $BASEDIR/config.yml.FREE.tmp
+  cat $BASEDIR/license.yml $BASEDIR/config.yml.FREE.tmp > $BASEDIR/config.yml
+  rm $BASEDIR/config.yml.FREE.tmp
 
-elif $highres; then
-  ($all || $lowres || $midres) && die "Cannot use option -h with options -a, -l or -m"
-  echo "Generating new config.yml file with high resources from config-2_1.yml"
-  patch -o $BASEDIR/config-2_1.yml.HIGHRES $BASEDIR/config-2_1.yml $BASEDIR/config-2_1.yml.high_res.patch
-  circleci config process $BASEDIR/config-2_1.yml.HIGHRES > $BASEDIR/config.yml.HIGHRES.tmp
-  cat $BASEDIR/license.yml $BASEDIR/config.yml.HIGHRES.tmp > $BASEDIR/config.yml
-  rm $BASEDIR/config-2_1.yml.HIGHRES $BASEDIR/config.yml.HIGHRES.tmp
+elif $paid; then
+  ($all || $free) && die "Cannot use option -p with options -a or -f"
+  echo "Generating new config.yml file for paid tier from config-2_1.yml"
+  patch -o $BASEDIR/config-2_1.yml.PAID $BASEDIR/config-2_1.yml $BASEDIR/config-2_1.yml.PAID.patch
+  circleci config process $BASEDIR/config-2_1.yml.PAID > $BASEDIR/config.yml.PAID.tmp
+  cat $BASEDIR/license.yml $BASEDIR/config.yml.PAID.tmp > $BASEDIR/config.yml
+  rm $BASEDIR/config-2_1.yml.PAID $BASEDIR/config.yml.PAID.tmp
 
 elif $all; then
-  ($lowres || $midres || $highres || $has_env_vars) && die "Cannot use option -a with options -l, -m, -h or -e"
-  echo "Generating new config.yml file with low resources and LOWRES/MIDRES/HIGHRES templates from config-2_1.yml"
-
-  # setup lowres
-  circleci config process $BASEDIR/config-2_1.yml > $BASEDIR/config.yml.LOWRES.tmp
-  cat $BASEDIR/license.yml $BASEDIR/config.yml.LOWRES.tmp > $BASEDIR/config.yml.LOWRES
-  rm $BASEDIR/config.yml.LOWRES.tmp
+  ($free || $paid || $has_env_vars) && die "Cannot use option -a with options -f, -p or -e"
+  echo "Generating new default config.yml file for free tier and FREE/PAID templates from config-2_1.yml"

Review Comment:
   May want to echo out the whole "make sure you commit the newly generated files <list_files_here> after running this command if you want them to persist"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org