Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support nanoc environments #859

Merged
merged 10 commits into from Aug 22, 2016

Conversation

barraq
Copy link
Contributor

@barraq barraq commented May 31, 2016

Description

Introduce support for nanoc environment and addresses #676.

  • nanoc command is updated to include --env argument
  • site config is overriden using the active environment if any and fallback to 'default'
  • if environments is provided, tmp directory is updated to tmp/{{env_name}}

Setting environments is done in nanoc.yaml using the environments property.
Example usage is:

output_dir: output

environments:
  default: &default
    base_url: ...
    ...
  development:
    <<: *default
    base_url: ...
  production:
    <<: *default
    base_url: ...
  yet_another_env:
    <<: *default
    base_url: ...
    output_dir: build

Selecting working environment can be done:

  • using environment variable NANOC_ENVIRONMENT
  • using nanoc --env=[<value>]

Progress

  • Main Nanoc command (cli/commands/nanoc.rb) is updated to include --env option
  • Nanoc::Int::Configuration is updated to provide with_environment(env = nil)
  • unless define in environment, output_dir is set as {{output_dir}}/{{env_name}}, if environments is not defined then output_dir remains unchanged
  • if environments is provided, tmp directory is updated to tmp/{{env_name}}
  • New tests are added

@barraq barraq changed the title Support nanoc environments [WIP] Support nanoc environments May 31, 2016
@barraq barraq force-pushed the feature/support-environments branch from 56d35ef to 98d5251 Compare May 31, 2016 15:10
@barraq
Copy link
Contributor Author

barraq commented May 31, 2016

Is missing:

  • handeling /tmp dir so that we use the same mechanism as the output_dir.
  • unit tests

Might be added:

  • introduce @env (@site.env) variable to access only active env variable

@gpakosz
Copy link
Member

gpakosz commented May 31, 2016

Does it take into account the cascading of configuration files (parent_config_file)?

@gpakosz
Copy link
Member

gpakosz commented May 31, 2016

Is <<: *default an inheritance mechanism?

@barraq
Copy link
Contributor Author

barraq commented May 31, 2016

Good point regarding parent_config_file. Actually the config is overridden by environment after it is loaded, therefore after it includes the parent_config_file: should be 🆗 then.
Regarding the <<: *default it is just some YAML property for inheritance indeed: https://en.wikipedia.org/wiki/YAML#References. You don't need to use it, I just did cause it is common practice.

@denisdefreyne
Copy link
Member

This looks cool! A couple of things that I spotted so far:

  • The #site_from_config method takes the configuration as an argument, and modifies it. I dislike modifying incoming arguments, because it makes code harder to follow and effects more difficult to trace.
  • Whatever’s in lib/nanoc/base can’t refer to anything in lib/nanoc/cli; the command-line interface knows about base, but not vice versa.

@@ -8,8 +8,9 @@ def new_with_config(hash)
site_from_config(Nanoc::Int::Configuration.new(hash).with_defaults)
end

def new_from_cwd
site_from_config(Nanoc::Int::ConfigLoader.new.new_from_cwd)
#def new_from_cwd(env = :default)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

oups... ➡️ will be removed

@barraq barraq force-pushed the feature/support-environments branch 2 times, most recently from c6ae4de to a1a43d5 Compare June 3, 2016 13:43
@barraq barraq changed the title [WIP] Support nanoc environments [MVP] Support nanoc environments Jun 7, 2016
@barraq barraq force-pushed the feature/support-environments branch from ad17ed7 to 8433fe5 Compare June 7, 2016 17:51
@barraq barraq changed the title [MVP] Support nanoc environments Support nanoc environments Jun 9, 2016
@barraq
Copy link
Contributor Author

barraq commented Jun 11, 2016

@ddfreyne do you need me to do any additional changes?

@denisdefreyne
Copy link
Member

@barraq I’ll give this a more thorough review soon!

@@ -47,6 +51,21 @@ def with_defaults
self.class.new(new_wrapped)
end

def with_environment(env = nil)
Copy link
Member

Choose a reason for hiding this comment

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

with_environment does not appear to be called with an argument except for tests. Make the tests set up the environment, and remove this argument.

@denisdefreyne
Copy link
Member

@barraq Is this PR still being worked on? I can take over if you wish.

@gpakosz
Copy link
Member

gpakosz commented Jul 27, 2016

BTW, how does this feature interact with nanoc view?

@denisdefreyne denisdefreyne modified the milestones: 4.3, 4.4 Aug 21, 2016
Introduce basic support for nanoc environment:
- nanoc command is updated to include --env argument
- config are overriden using the active environment if any
- unless define in environment, output_dir is {{output_dir}}/{{env_name}}

Setting environments is done in nanoc.yaml using the `environments` property.
Example usage is:

```
output_dir: output

environments:
  default: &default
    base_url: ...
    ...
  development:
    <<: *default
    base_url: ...
  production:
    <<: *default
    base_url: ...
  yet_another_env:
    <<: *default
    base_url: ...
    output_dir: build
```

Selecting working environment can be done:
- using environment variable `NANOC_ENVIRONMENT`
- using `nanoc --env=[<value>]`
- Document env attr_reader
- Only allow String and nil for environment name
- Remove argument for with_environment
- Make :environments a constant
- Lot of parentheses fix
- Renamed NANOC_ENVIRONMENT to NANOC_ENV for consistency with Rails, Rake, etc.
- Prefer Fetch(a,b) over a||b
- Refactor tmp_path logic into Nanoc::Int::Store
- Make -e, --env argument required
- Updated test according to previous changes
@barraq barraq force-pushed the feature/support-environments branch from 4a29948 to 8676556 Compare August 21, 2016 08:18
- add Contracts
- rename store to store_name
- use named arguments
@barraq barraq force-pushed the feature/support-environments branch from 220fe03 to e208d05 Compare August 21, 2016 08:49
@barraq barraq force-pushed the feature/support-environments branch from e065bf5 to 6e67994 Compare August 21, 2016 09:39
@barraq
Copy link
Contributor Author

barraq commented Aug 21, 2016

@ddfreyne I went over all fixes you mentioned. I did small fixup commits to ease the review process.
@gpakosz support for environments allows you to override the @site.config. You can then use it however you want: e.g. filter (within your Rules file) draft items in production environment; display additional things in templates for a given environments; etc.

@denisdefreyne
Copy link
Member

Looks good! Thanks for this feature. Merging!

@denisdefreyne denisdefreyne merged commit d529a87 into nanoc:master Aug 22, 2016
@bburton
Copy link

bburton commented Aug 23, 2016

As much as I like the idea of adding environment support, this change makes certain assumptions that will cause problems with certain filters. For instance, the compass filter creates a .sass-cache folder in the project directory where nanoc compile is run. It's quite possible other filters may create temporary folders where files are cached.

As a result, the only safe way to use this feature is to only compile to one environment in a given project folder and not compile different environments in the same project folder.

Sure it's possible to configure the sass compiler to cache to a specific location but the setup is complicated because the environment setting needs to be passed to the compass config.rb and used to set the sass :cache_location property. Not only that, the configuration has to be made to config.rb so it's possible to run the compass command by itself outside of nanoc and have it pick up the right environment location.

Compass also supports environments so it would be ideal if the environment setting configured for Nanoc could be passed into the compass config.rb but that's a bit beyond the current topic.

However, aside from the filters nanoc supports out of the box, even if all of them were reviewed and fixed if necessary, there's no guarantee that third-party filters aren't writing or caching files to the project directory.

Even if switching environments to compile into the same folder is not supported, environment support is still very useful when building with one environment configuration such as development in its own folder, then push changes to a Git repository, e.g. github.com, then in a different folder configured as the production environment perform a git pull and recompile.

@denisdefreyne
Copy link
Member

@bburton Thanks for the comment! I’ve collected your thoughts in #937.

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.

None yet

4 participants