Skip to content

Conversation

@raupach
Copy link

@raupach raupach commented Jun 21, 2016

In my opinion, the configuration and the calculated values should be clearly separated.

I have done the following:

  • project.config.ts is now an abstract base class of seed.config.ts
  • all configuration-values moved to project.config.ts
  • seed.config.ts holds the calculated values.
  • I split up some values. Example: CSS_DEST = ${this.APP_DEST}/${this.CSS_DEST_DIR};`

@TheDonDope
Copy link
Contributor

Hi @raupach,
thank you for the PR. I can see your motivation behind the separation of the calculated values, however reversing the class hierarchy would break the general semantic division we have for the tooling (say all seed provided tasks residing in tools/tasks/seed, and project specific ones in tools/tasks/project; same for utils etc.). Therefore i would rather not stray away from this schema for the config part.

@lifaon74
Copy link

I join @raupach

This is painfull than attribute values are computed at the instanciation of seed.config.ts. This implies we need to copy ALL the attributes from seed.config.ts to project.config.ts before changind their values... From my point of view, all computed values should be placed into methods.

@mgechev
Copy link
Owner

mgechev commented Jun 23, 2016

We're discussing the best way to implement such computed properties in this PR #968. We'd love to hear another opinion.

@raupach
Copy link
Author

raupach commented Jun 23, 2016

Hi @TheDonDope,

but there, nothing has changed - except that project.config.ts holds ALL the configuration-values.
Lets say, project.config.ts holds all the default values and the individual project can now change these values.

The calculation of further values takes place AFTER "reading" project.config.ts at seed.config.ts.
And - this is important - seed.config.ts did not hold additional constants. For Example:

before:
seed.config.ts: CSS_SRC = ${this.APP_SRC}/css;

after:
project.config.ts: CSS_DIR = 'css';
CSS_SRC = ${this.APP_SRC}/${this.CSS_DIR};

Thes application is now highly configurable and with a little more calculation even empty values are possible.

@mgechev
Copy link
Owner

mgechev commented Jul 4, 2016

There's merge conflict. Lets discuss the best way to solve #823 here #968.

@mgechev mgechev closed this Jul 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants