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

Already on GitHub? Sign in to your account

The Big Jekyll Command Cleanup #690

Merged
merged 22 commits into from Jan 12, 2013

Conversation

Projects
None yet
4 participants
Contributor

tombell commented Dec 16, 2012

So I've decided to dedicate some time to cleaning up the bin/jekyll binary.

This branch is pretty much finished.

Commands are more intuitive thanks to the useful help output. There are three distinct subcommands at the minute; build, serve and import.

Build

Builds the jekyll site, has some options:

  • -w, --watch will watch for changes and rebuild when they're detected
  • --lsi will build using LSI for improved related posts

Serve

Serves the built jekyll site, has some options:

Passed to the pre build command before serving

  • -w, --watch will watch for changes and rebuild when they're detected
  • --lsi will build using LSI for improved related posts

Passed to the serve command for serving

  • -p, --port port to listen on
  • -h, --host host to bind to
  • -b, --baseurl base URL

Import/Migrate

Imports blog posts from another platform using the migrators.

  • --file file to migrate from
  • --dbname database name to migrate from
  • --user user to use when migrating
  • --pass password to use when migrating
  • --host host address to use when migrating
Owner

parkr commented Dec 18, 2012

I mentioned this on your Gist, but I wanted to chime in here as well.

How would the use of the Commander gem support multiple commands? For example, I often run both --auto and --server. With your configuration, I'd have to either run jekyll serve --auto or jekyll watch --serve.

It seems like modularizing commands in that way will cause confusion.

Contributor

tombell commented Dec 18, 2012

jekyll build --watch or jekyll serve --watch

To me the automatic rebuilding isn't a command it's an option of the building or serving. I updated my gist last night https://gist.github.com/4315072

tombell added some commits Dec 18, 2012

@tombell tombell Add bin/jekyll2 and initial BuildCommand
The `BuildCommand` class is responsible for handling the building of the site.
It can also optionally watch for changes to files and regenerate the site if
needed.

The `Command` class holds any methods which are used by any command
implementation.
1476649
@tombell tombell Add initial serve command
The `ServeCommand` will let you serve your site locally for development. You
can specify `--port`, `--host` and `--baseurl` options if you wish to change the
defaults.

Additionally the `BuildCommand` will be called before the processing of the
serve command, this makes sure that the site is actually built. This means you
are able to pass the `--watch` option to auto-regenerate your site, even while
serving it locally.
3b4feb4

@parkr parkr commented on the diff Dec 18, 2012

lib/jekyll/command.rb
@@ -0,0 +1,14 @@
+module Jekyll
+
+ class Command
@parkr

parkr Dec 18, 2012

Owner

Make sure you TomDoc that :)

@tombell

tombell Dec 18, 2012

Contributor

I'm following by example... only certain methods in jekyll are actually TomDoc'd, not everything.

@parkr

parkr Dec 22, 2012

Owner

Tom mentioned that he wanted to TomDoc everything that's in the repo right now – he just hasn't gotten to it, yet.

@parkr parkr commented on the diff Dec 18, 2012

lib/jekyll/commands/build.rb
@@ -0,0 +1,82 @@
+module Jekyll
+
+ class BuildCommand < Command
@parkr

parkr Dec 18, 2012

Owner

All of these classes and methods, actually :)

Do you have a readme outlining the command structure and switches?

Owner

parkr commented Dec 18, 2012

Nicely done!

Contributor

tombell commented Dec 19, 2012

Swapped over the bin/jekyll in features/support/env.rb to bin/jekyll2 build, and every still passes.

Now is the time to refine everything :shipit:

@tombell tombell Remove command options from default config
Removing command line options from the config is a path towards cleaning up the
configuration file and not including options which don't really belong there.
a151a16

@parkr parkr referenced this pull request Dec 22, 2012

Closed

Clean Up bin/jekyll #609

@parkr parkr referenced this pull request Jan 2, 2013

Closed

Jekyll Sub-commands #726

Owner

parkr commented Jan 11, 2013

  • Put migrators back (we're splicing them out later)
  • Rename :migrate command to :import
Contributor

tombell commented Jan 11, 2013

Updated main PR comment with more details.

@parkr parkr commented on the diff Jan 11, 2013

lib/jekyll.rb
'source' => Dir.pwd,
'destination' => File.join(Dir.pwd, '_site'),
+

@parkr parkr commented on the diff Jan 11, 2013

lib/jekyll.rb
@@ -120,6 +119,9 @@ module Jekyll
#
# Returns the final configuration Hash.
def self.configuration(override)
+ # Convert any symbol keys to strings and remove the old key/values
+ override = override.reduce({}) { |hsh,(k,v)| hsh.merge(k.to_s => v) }
Owner

parkr commented Jan 11, 2013

Thanks for making those changes! One last thing is to write TomDocs for everything and I'll merge this in. Thanks :)

Owner

parkr commented Jan 12, 2013

I can't automatically merge this - rebase on current master? Thanks @tombell! Merging this in once it's ready.

Owner

mattr- commented Jan 12, 2013

You could merge it manually, you know. That's how we did it back in the old
days. 😉

On Friday, January 11, 2013, Parker Moore wrote:

I can't automatically merge this - rebase on current master? Thanks
@tombell https://github.com/tombell! Merging this in once it's ready.


Reply to this email directly or view it on GitHubhttps://github.com/mojombo/jekyll/pull/690#issuecomment-12170726.

Owner

parkr commented Jan 12, 2013

In the new days, we prefer GitHub's built-in merge functionality. 😉 But I get ya.

I'll merge this in tonight -- I'm not at my computer at the moment. Thanks for your hard work, @tombell!

Owner

mattr- commented Jan 12, 2013

No doubt. The GitHub built-in merge functionality is preferred. I was just giving you shit really. 😄

@parkr parkr added a commit that referenced this pull request Jan 12, 2013

@parkr parkr Merge pull request #690 from tombell/jekyll-command-redux
The Big Jekyll Command Cleanup
14cabab

@parkr parkr merged commit 14cabab into jekyll:master Jan 12, 2013

@ghost ghost assigned parkr Jan 12, 2013

Owner

parkr commented Jan 12, 2013

Changing --auto to --watch is going to be confusing if the _config.yml files contain auto. Switch auto to watch in site configuration?

@mojombo, @qrush?

Owner

parkr commented Jan 12, 2013

We should also consider removing the server switch from _config.yml.

Owner

parkr commented Jan 12, 2013

I'm updating the docs and relevant options which were missed for build and serve. PR soon.

@tombell tombell deleted the unknown repository branch Jan 12, 2013

@jekyllbot jekyllbot locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.