Blueprintrc #117

anithri · 2017-09-26T03:06:54Z

This is built on top of a rename PR that's paused while we reimplement the cli

But there;s stuff to checkout.

models/project-settings

What this PR does is expand the ProjectSettings to allow it to accumulate arrays of blueprint paths, and relate them to the .blueprintrc file it came from. Other changes make is possible to track specific settings to the .blueprintrc file it came from.

models/blueprint-collection

The Other addition is models/blueprint-collection. This is a finder, loader and organizer of blueprints. It's currently instantiated by ProjectSettings, but it wouldn't take much to convice me that should actually happen in sub-command instead.

Right now it will get blueprint paths from settings, and provide a searchPaths that is an array of all of the valid blueprint paths. It also adjusts paths relative to home(~), absolute(/) and relative to the blueprintrc dir

Next up will be removing moving things from Blueprint and BlueprintCollection. Collection to be in charge of finding, loading, collecting and indexing. Blueprint doesn't have to care about anything but the blueprint at the path given

anithri · 2017-09-26T03:13:39Z

One odd thing you might note.
the config and blueprint chunks are .unshifted into their respective arrays. This is beacuse of the way rc reads configurations. It starts with the location that every other location will override, and then proceeds to read in order getting closer and more important to the current path. As it does it populates a key in the settings object called "configs" which in array containing the full path of every config it read.

But for blueprints purposes, the blueprint paths from the last chunk read are the ones we will check first,
Therefore we unshift so the new chunk goes at the beginning of the list, and we can iterate through it in the correct order of precedence.

anithri · 2017-09-26T03:24:24Z

⁉️ Referencing NPM modules as blueprint source ⁉️

I think this is an important feature. It will promote sharing of blueprints which leads to better blueprints which benefits everybody.

the brute force way to do this is to check every dir containing a .blueprintrc file for a node_modules, under that look for a dir that matches the given name and that has a blueprints subdir.

nicer way. add a syntax element like @blueprints-firebase, and look in the node_modules directories for those. Could refine and use npm to inform us which dirs it's would search for that package name, and only check those.

Are runtime npm requires possible? could we just require the module and get the blueprint object back?

Other ideas?

anithri · 2017-09-26T03:39:55Z

`#loadRunnable()` changed to `#generators()`

@jamesr73, this will return an array of instantiated Blueprints that will work for the generate cmd

There will also be: #all() and #partials(), and assets will join eventually

anithri · 2017-09-26T03:41:43Z

note

currently the ProjectSettings obj instantiates a blueprint-collection at settings.blueprints This could be moved to be a property of the SubCommand obj

jamesr73 · 2017-09-26T08:37:38Z

src/models/blueprint-collection.js

+    this.settings = settings;
+    this.setSearchPaths();
+  }
+


Seeing 'settings' here was a mental disconnect for me, my first thought was that it was the ProjectSettings object, which it isn't, only the configured search paths.

good point. I'll rename that to something like rawPaths or potentialPaths

jamesr73 · 2017-09-26T08:38:52Z

src/models/blueprint-collection.js

+    // in any case we need to return an instantiated Blueprint here
+    return path;
+  }
+


I suggest we keep Blueprint#load the responsibility of Blueprint so it handles the mixin logic. And move any discovery, lookup, list functionality into here.

I think you're right. I'll implement it that way.

jamesr73 · 2017-09-26T08:53:58Z

Runtime requires are definitely possible, that's essentially how the CLI is built out. We'd just need to structure the blueprint plugin modules appropriately so they provide a simple means to get at their list of blueprints.

List of blueprint paths vs instantiated blueprints? To avoid potential version conflicts in the Blueprint implementation?

That would give an intuitive search path for modules vs explicit paths in rc files.

anithri · 2017-09-26T21:46:04Z

It should be easy for the entry point of the blueprint package to be a simple function that returns a list of absolute paths to all of the blueprints in it's local blueprints dir.

Then BlueprintCollection can handle the loading of those paths just like any other resolved path.

And I think if we have to require them, we'll definately need some token that indicates that any particular entry is an npm package. Otherwise we'll be trying to require things that may or may not exist as npm

jamesr73 · 2017-09-26T23:22:30Z

I feel a blueprint for creating blueprint modules is on the horizon...

Agree that the modules need a simple marker in the blueprint paths so we know to require them. Might @ be confusing as it's used to scope packages, so including a scoped package would need @@scope/package.

module:blueprint-saga
extend:blueprint-saga

renamed source and test files changed require paths ignore references to repo, actual redux references and other projects's names changed a lot of references to .reduxrc

added npm bin definitions to allow "bp" as command

Currently only returns the paths to blueprints.

removed unneeded bin files add a project level .blueprintrc and removed it from .gitignore

increased Blueprint.js test coverage a bit

blueprintChunks count was including my ~/.blueprintrc but travis-ci didn't have that so count was different

anithri · 2017-10-01T03:39:58Z

I think this about ready to merge. @jamesr73 could you give it a review?

renamed to .ejs until we can figure out a fix

codecov-io · 2017-10-01T06:12:29Z

Codecov Report

Merging #117 into master will increase coverage by 3.08%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #117      +/-   ##
==========================================
+ Coverage   76.33%   79.42%   +3.08%     
==========================================
  Files          21       22       +1     
  Lines         262      311      +49     
==========================================
+ Hits          200      247      +47     
- Misses         62       64       +2

Impacted Files	Coverage Δ
test/fixtures/blueprints/duplicate/index.js	`100% <100%> (ø)`
lib/models/sub-command.js	`100% <0%> (ø)`	⬆️
lib/models/project-settings.js	`100% <0%> (ø)`	⬆️
lib/config.js
lib/models/blueprint-collection.js	`93.75% <0%> (ø)`
lib/cli/cmds/generate/build-blueprint-commands.js	`53.84% <0%> (+3.84%)`	⬆️

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 55d90dd...57c45de. Read the comment docs.

anithri · 2017-10-01T07:26:56Z

src/sub-commands/config.js

@@ -0,0 +1,47 @@
+import figlet from 'figlet';


Partially addresses #107

jamesr73

Looks good. Only minor comments/questions.

jamesr73 · 2017-10-01T11:06:44Z

package.json

@@ -11,7 +11,7 @@
    "posttest": "npm run lint",
    "test:nocov": "jest --coverage=false --forceExit",
    "test:cov": "jest --forceExit",
-    "test:watch": "jest --forceExit --watch --notify",
+    "test:watch": "jest --forceExit --coverage=fals --watch --notify",


Typo? I was using this locally:

jest --forceExit --coverageReporters html --watch --notify

Gives a much shorter coverage summary in the jest console but also give access to a local report if you want to read it.

I'll give that a try. I was getting frustrated when the test fails would be pushed off the screen by the full coveragae report

jamesr73 · 2017-10-01T11:14:37Z

src/models/blueprint-collection.js

+  return final;
+}
+
+export function parseSetting (setting) {


I can't see where parseSetting is used?

yeah...

So that function is supposed to take the list of blueprint paths for a given .blueprintrc file and add the ./blueprints directory to the end of that list.

I believe I wrote it and then became unsure of where it should go. Let me fix that.

Well it is actually being used.

it's imported in models/project-settings and had it's name changed to parseBlueprint. which is used in the Rc parser wrapper. It's job is to turn whatever the settings are into arrays, and to add ./blueprints to the end of those arrays. this lets you do

It's defined in BlueprintCollection because that's the concerned party
It's called in ProjectSettings because that's where that job needs to place

I changed the name to parseBlueprintSetting for both usages.

If you're ok with this, I'm going to merge

jamesr73 · 2017-10-01T11:25:29Z

src/models/project-settings.js

-    // if the config file list is empty, and the config is only the default
-    // maybe save or prompt to save the default config file
+    const startingSettings = Object.assign({}, this.defaultSettings);
+    this.settings = rc('blueprint', startingSettings, this.args, this.myParse);


Does rc modify startingSettings? Might we have an issue with values in this.defaultSettings being modified since the values will be shared by startingSettings.

Yes it does. And yes we will or at least might.

default settings aren't being used right now, but we are going to need to use them. We need the default settings to have whatever settings we need to have in order to run the CLI in the complete absence of .blueprintrc files. It also needs to add our package blueprints directory.

The only real reason to maintain the integrity of the defaultSettings is to be able display just those settings, which is useful for troubleshooting complex configs.
if we keep it then change to:
startingSettings = JSON.parse(JSON.stringify(defaultSettings))?

Sure. Quick and easy. If it ever becomes problematic could migrate to a deep copy.

jamesr73 · 2017-10-01T11:28:40Z

src/sub-commands/config.js

+    console.log(prettyjson.render(this.settings.blueprints.allNames(), {}, 8));
+  }
+
+  cliLogo () {


cliLogo is duplicated in sub-commands/init.js. Perhaps move it up into SubCommand.

right you are you

tweaked test:watch in package.json

some minor file formatting

jamesr73 · 2017-10-01T17:43:22Z

👍

* 'master' of https://github.com/SpencerCDixon/redux-cli: Bug bp cmd (SpencerCDixon#123) Blueprintrc (SpencerCDixon#117)

jamesr73 reviewed Sep 26, 2017

View reviewed changes

anithri added 12 commits September 30, 2017 18:51

rename to blueprint.

277c20d

renamed source and test files changed require paths ignore references to repo, actual redux references and other projects's names changed a lot of references to .reduxrc

renamed bin files to blueprint

6eaaaf8

added npm bin definitions to allow "bp" as command

first pass

96b2072

first pass

3d73eca

Start of BlueprintCollection

c4da77c

add #all() and supporting cast

cdb9487

Currently only returns the paths to blueprints.

update .gitignor

92efd2d

cleanup after big rebase.

52db4e0

removed unneeded bin files add a project level .blueprintrc and removed it from .gitignore

Config Command, BlueprintCollection, Testing, linting

6a07133

refactor Blueprint.loadRunnable to BlueprintCollection#allGenerators()

71f630e

increased Blueprint.js test coverage a bit

BlueprintCollection.generators not .allGenerators

d2c529c

Fixed test

3d7633e

blueprintChunks count was including my ~/.blueprintrc but travis-ci didn't have that so count was different

Noping eslint ejs mess

9883241

renamed to .ejs until we can figure out a fix

anithri commented Oct 1, 2017

View reviewed changes

src/sub-commands/config.js Outdated

@@ -0,0 +1,47 @@

import figlet from 'figlet';

Copy link

Collaborator Author

anithri Oct 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially addresses #107

jamesr73 reviewed Oct 1, 2017

View reviewed changes

anithri added 3 commits October 1, 2017 10:58

Move .cliLogo() into models/sub-command

328dc23

tweaked test:watch in package.json

rename parseSettings to parseBlueprintSettings

7a90aac

some minor file formatting

duplicate defaultSettings before passing them to rc

57c45de

anithri merged commit 867a1eb into SpencerCDixon:master Oct 1, 2017

anithri deleted the blueprintrc branch October 1, 2017 20:23

jamesr73 added a commit to jamesr73/redux-cli that referenced this pull request Oct 2, 2017

Merge branch 'master' of https://github.com/SpencerCDixon/redux-cli

61120e7

* 'master' of https://github.com/SpencerCDixon/redux-cli: Bug bp cmd (SpencerCDixon#123) Blueprintrc (SpencerCDixon#117)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Blueprintrc #117

Blueprintrc #117

anithri commented Sep 26, 2017

anithri commented Sep 26, 2017

anithri commented Sep 26, 2017 •

edited

anithri commented Sep 26, 2017

anithri commented Sep 26, 2017

jamesr73 Sep 26, 2017

anithri Sep 26, 2017

jamesr73 Sep 26, 2017

anithri Sep 26, 2017

jamesr73 commented Sep 26, 2017

anithri commented Sep 26, 2017

jamesr73 commented Sep 26, 2017 •

edited

anithri commented Oct 1, 2017

codecov-io commented Oct 1, 2017 •

edited

anithri Oct 1, 2017

jamesr73 left a comment

jamesr73 Oct 1, 2017

anithri Oct 1, 2017

jamesr73 Oct 1, 2017

anithri Oct 1, 2017

anithri Oct 1, 2017

anithri Oct 1, 2017

jamesr73 Oct 1, 2017

anithri Oct 1, 2017 •

edited

jamesr73 Oct 1, 2017

jamesr73 Oct 1, 2017

anithri Oct 1, 2017

jamesr73 commented Oct 1, 2017

Blueprintrc #117

Blueprintrc #117

Conversation

anithri commented Sep 26, 2017

models/project-settings

models/blueprint-collection

anithri commented Sep 26, 2017

anithri commented Sep 26, 2017 • edited

⁉️ Referencing NPM modules as blueprint source ⁉️

anithri commented Sep 26, 2017

#loadRunnable() changed to #generators()

anithri commented Sep 26, 2017

note

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

jamesr73 commented Sep 26, 2017

anithri commented Sep 26, 2017

jamesr73 commented Sep 26, 2017 • edited

anithri commented Oct 1, 2017

codecov-io commented Oct 1, 2017 • edited

Codecov Report

Choose a reason for hiding this comment

jamesr73 left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anithri Oct 1, 2017 • edited

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

jamesr73 commented Oct 1, 2017

anithri commented Sep 26, 2017 •

edited

`#loadRunnable()` changed to `#generators()`

jamesr73 commented Sep 26, 2017 •

edited

codecov-io commented Oct 1, 2017 •

edited

anithri Oct 1, 2017 •

edited