next take on updated rc system #132

anithri · 2017-10-25T07:55:37Z

// src/cli/environment.js
...
if (!environment) {
  const configCli  = argv.getOption('config'); // or whatever
  const defaults   = fetchDefaults(); // or whatever
  const rcFiles    = new FileCollection(configCli);
  const rcRaw      = new RcRaw(rcFiles.present, defaults);
  const rc         = new RcData(rcRaw.data);
  const bpDirs     = new DirCollection(rcFiles, rc.blueprintPaths)
  const blueprints = new BlueprintCollection(bpDirs.present, rc);
  const ui         = new UI();
  environment = {
    rcFiles,
    rcRaw,
    rc,
    bpDirs,
    blueprints,
    ui
  };

FileCollection

rcFiles.all returns every file path checked for an rc file
rcFiles.present returns paths to all files that exist

RcRaw

rcRaw.collection an object with keys of the filename and values of the contents of that file
rcRaw.order an array of rcFile names in the order of priority
rcRaw.data an array of rcFile contents in order of of priority

RcData

rc.settings object created by deep merge of all file contents
rc.get('key.path.parts[4]', default) returns that value from the settings or default
rc.with(opts) returns a new rc object created from itself and opts
rc.withBp(name,opts) returns a new rc object for that Blueprint, merged with common settings

DirCollection

bpDirs.all returns every directory that might be checked for blueprints
bpDirs.present returns paths to all directoryies that exist

codecov-io · 2017-10-25T07:55:40Z

Codecov Report

Merging #132 into master will increase coverage by 0.88%.
The diff coverage is 25%.

@@            Coverage Diff             @@
##           master     #132      +/-   ##
==========================================
+ Coverage   82.17%   83.05%   +0.88%     
==========================================
  Files          27       30       +3     
  Lines         387      413      +26     
==========================================
+ Hits          318      343      +25     
- Misses         69       70       +1

Impacted Files	Coverage Δ
test/fixtures/blueprints/basic/index.js	`25% <25%> (-75%)`	⬇️
lib/tasks/git-pull.js	`14.28% <0%> (-3.11%)`	⬇️
lib/tasks/create-and-step-into-directory.js	`14.28% <0%> (-0.72%)`	⬇️
lib/models/sub-command.js	`100% <0%> (ø)`	⬆️
lib/cli/environment.js	`100% <0%> (ø)`	⬆️
lib/models/project-settings.js
test/fixtures/basic/index.js
lib/models/dir-collection.js	`100% <0%> (ø)`
lib/util/mergers.js	`20% <0%> (ø)`
lib/models/file-collection.js	`100% <0%> (ø)`
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 00684d6...e392240. Read the comment docs.

jamesr73 · 2017-10-27T16:45:47Z

Looks great. I've been pondering the integration of argv.getOption('config')...

I think it makes sense to ditch the environment singleton and simply call getEnvironment(cliConfig). The singleton was introduced (by me) to ensure that each of the CLI commands didn't cause config/blueprints to be loaded multiple times whilst building out the CLI. E.g.

// cil/cmds/init.js

import getEnvironment from '../environment';
import Init from '../../sub-commands/init';

const subCommand = new Init(getEnvironment());

const usage = 'Usage:\n  $0 init';

module.exports = {
  command: 'init',
  describe: 'Initialize .blueprintrc for the current project',
  builder: yargs => yargs.usage(usage),
  handler: () => subCommand.run()
};

If we move the instantiation of the environment inside the handler we have access to the cli config argument and we're guaranteed to only create a single environment in the context of any command. This is already the case for the generate command.

// cli/cmds/init.js

import getEnvironment from '../environment';
import Init from '../../sub-commands/init';

const usage = 'Usage:\n  $0 init';

module.exports = {
  command: 'init',
  describe: 'Initialize .blueprintrc for the current project',
  builder: yargs => yargs.usage(usage),
  handler: argv => {
    const environment = getEnvironment(argv.config);
    const subCommand = new Init(environment);
    subCommand.run();
  }
};

We could choose to pass argv.config or the entire argv, which would give more opportunity to integrate CLI arguments directly into the environment.

I'm happy to provide a PR that would make those changes to the CLI commands (leaving that actual changes to getEnvironment to this PR).

anithri · 2017-10-29T19:44:59Z

I've been accounting for taking the argv, but have purposely not spelled anything out about how and when.

I think the only real 'danger' of not using a singleton requires using it twice within the same invocation. The IO cycles slow it down, but not noticably. So where ever in the process we decide to read it, we just need to make sure it's not read a second time. We could add tests to ensure that, but i'm not sure it's worth even that small amount of effort.

The CLI pattern you laid out looks good to me.

I'm working on the rest of the rc process today.

Stay tuned to this space for all the latest

anithri · 2017-10-29T21:14:47Z

More thoughts.

I want to maintain a good separation of concerns between the classes. I'm thinking about this

{ 
 // ...
 handler: argv => {
    const environment = getEnvironment(argv); // argv being the result of parsing ENV and cli options
    const subCommand = new Init(environment);
    subCommand.run();
  }
}

function getEnvironment (args) {
  const name = args.rcName; // or whatever
  const configFiles  = args.config; // or whatever
  const defaults   = fetchDefaults(); // or whatever
  const rcFiles = new FileCollection({configFiles, name});
  const rcRaw = new RcRaw({rcFiles: rcFiles.present, args, defaults});
  const rc = new RcData(rcRaw.data); // WIP
  const bpDirs = new DirCollection(rcFiles, rc.blueprintPaths, {name});
  const blueprints = new BlueprintCollection(bpDirs.present, rc);
  const ui = new UI();
  environment = {
    rcFiles,
    rcRaw,
    rc,
    bpDirs,
    blueprints,
    ui
  };

jamesr73 · 2017-10-29T22:19:10Z

Using the CLI pattern as described means there will only ever be a single call to instantiate the CLI environment using argv. It will then be passed into the SubCommand which can in turn pass it into any Tasks it needs to run. I'll submit a PR with those changes.

anithri · 2017-10-30T00:55:48Z

I'm much happier with this take. I reduced the ridiculous amount of DI I was doing and scaled it back to a few targeted places where an override for the ENV or fs made testing much easier.

Next step is the hand off of blueprint paths and then instantiating the Blueprint Collection.

anithri · 2017-11-02T02:12:50Z

Victory is Mine! 🏆

[scottp:~/4winds/redux-cli] rcTake2+ 3s ± bp config                            
______ _                       _       _          _____  _     _____ 
| ___ \ |                     (_)     | |        /  __ \| |   |_   _|
| |_/ / |_   _  ___ _ __  _ __ _ _ __ | |_ ______| /  \/| |     | |  
| ___ \ | | | |/ _ \ '_ \| '__| | '_ \| __|______| |    | |     | |  
| |_/ / | |_| |  __/ |_) | |  | | | | | |_       | \__/\| |_____| |_ 
\____/|_|\__,_|\___| .__/|_|  |_|_| |_|\__|       \____/\_____/\___/ 
                   | |                                               
                   |_|                                               
  info: Rc Files
        - /home/scottp/4winds/redux-cli/.blueprintrc
        - /home/scottp/.blueprintrc
        - /home/scottp/.config/.blueprintrc
        - /home/scottp/.config/blueprint/blueprintrc
  info: Rc Data
          sourceBase: src
          testBase:   test
          smartPath:  container
          dumbPath:   component
          fileCasing: default
          location:   project
          blueprints: 
            (empty array)
          assembly: 
            rcFiles: 
              - /home/scottp/4winds/redux-cli/.blueprintrc
              - /home/scottp/.blueprintrc
              - /home/scottp/.config/.blueprintrc
              - /home/scottp/.config/blueprint/blueprintrc
  info: Blueprint Paths
        - /home/scottp/4winds/redux-cli/blueprints
        - /home/scottp/.blueprints
  info: Blueprints
        - blueprint
        - duck
        - dumb
        - form
        - smart
        - woot

anithri · 2017-11-02T02:20:23Z

I broke a ton of tests, and I'll fix those this weekend. But it's reading rc files and finding and instantiating blueprints.

The big areas that need addressed are.

complete loop of parsed env & cli added to rc
figure out syntax for using an npm module as a blueprint path
- Use an rc entry like `blueprintPackages' that is only used to refer to npm modules
- Use a pattern like blueprint-set-redux-auto and just scan node_modules for everything matching blueprint-set.
- use a syntax like @myNodeModule to indicate a pkg, so it can have the path expanded to the correct location
At what point in a Blueprint's life cycle do we merge the rc data with that blueprints defaults and settings

anithri · 2017-11-06T21:09:41Z

I'm still happy with the way this came together, and I handled the test coverage too.

@jamesr73 can you give it a review when you have the time? Thanks.

start with the file collection

2e53020

tweaking file master a bit

d33a41d

anithri mentioned this pull request Oct 29, 2017

[WIP] Feature/blueprint clone command #133

Open

4 tasks

started with public getEnvironment(args = {})

be2cea8

anithri added 4 commits October 29, 2017 17:56

RcRaw & support

c84efdd

RcData & support

493184b

file-collection tests

c117e96

prettier

80af371

through to blueprints

40db5ab

clean up

5b88c0f

anithri mentioned this pull request Nov 2, 2017

Add Models - RcCollectionRaw and RcCollection #131

Closed

anithri added 3 commits November 6, 2017 13:33

tests for changes

32dabcb

pretty

a664b06

more test work

992b1bc

more tests

e392240

anithri closed this by deleting the head repository Jul 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

next take on updated rc system #132

next take on updated rc system #132

anithri commented Oct 25, 2017 •

edited

Loading

codecov-io commented Oct 25, 2017 •

edited

Loading

jamesr73 commented Oct 27, 2017 •

edited

Loading

anithri commented Oct 29, 2017

anithri commented Oct 29, 2017 •

edited

Loading

jamesr73 commented Oct 29, 2017

anithri commented Oct 30, 2017

anithri commented Nov 2, 2017

anithri commented Nov 2, 2017

anithri commented Nov 6, 2017

next take on updated rc system #132

next take on updated rc system #132

Conversation

anithri commented Oct 25, 2017 • edited Loading

FileCollection

RcRaw

RcData

DirCollection

codecov-io commented Oct 25, 2017 • edited Loading

Codecov Report

jamesr73 commented Oct 27, 2017 • edited Loading

anithri commented Oct 29, 2017

anithri commented Oct 29, 2017 • edited Loading

jamesr73 commented Oct 29, 2017

anithri commented Oct 30, 2017

anithri commented Nov 2, 2017

anithri commented Nov 2, 2017

anithri commented Nov 6, 2017

anithri commented Oct 25, 2017 •

edited

Loading

codecov-io commented Oct 25, 2017 •

edited

Loading

jamesr73 commented Oct 27, 2017 •

edited

Loading

anithri commented Oct 29, 2017 •

edited

Loading