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

remove ruby, browsersync, linting, pretty much everything #14

Merged
merged 22 commits into from
Mar 27, 2016

Conversation

labbydev
Copy link
Contributor

@labbydev labbydev commented Mar 2, 2016

See full notes here

Current Functionality

building sculpin and compiling sass.

Removes:

  • js linting
  • Browsersync
  • Ruby
  • sitespeed testing

Includes:

  • LibSass
  • autoprefixer
  • npm-breakpoint
  • zen-grids
  • npm-shrinkwrap
  • stylelint
  • bem-selector linter
  • deployment to gh-pages

Testing Instructions:

  • pull Butler into a project skeleton and place it in the project root
  • vagrant up
  • vagrant ssh
  • cd /var/sites/{project}/butler
  • npm install -g gulp
  • npm install
  • gulp
  • in a browser navigate to http://{project}.local:8000/
  • You should see the styleguide there
  • make some changes in your text editor to sass or views
  • refresh browser
  • Observe your styleguide has been updated
  • Write some "incorrect" sass (example: body { background:darkseagreen; }
  • Observe there are warnings that appear in your terminal

@patrickfweston
Copy link
Contributor

The directions seem to be missing the step that you need to run npm install after you change into the butler directory.

Also, when trying to run gulp, I get the message No command 'gulp' found, did you mean:. Running npm install gulp still resulted in the previous message. I was able to install gulp and get it running using sudo npm install -g gulp, although this may not be the correct way to do so. Running npm install -g gulp without sudo privileges did not work for me. I got an EACCESS error.

Also, once gulp started running, I get the following error:

events.js:85
      throw er; // Unhandled 'error' event
            ^
Error: Command failed: /bin/sh -c sculpin generate --watch --server
/bin/sh: 1: sculpin: not found

    at ChildProcess.exithandler (child_process.js:744:12)
    at ChildProcess.emit (events.js:110:17)
    at maybeClose (child_process.js:1008:16)
    at Socket.<anonymous> (child_process.js:1176:11)
    at Socket.emit (events.js:107:17)
    at Pipe.close (net.js:476:12)

If I follow the steps on this page (https://sculpin.io/download/) to install sculpin, then I can run gulp. One caveat is that in the last step mentioned on that page (mv sculpin ~/bin/), should be sudo mv sculpin /usr/bin for us.

I did try running composer install, but sculpin isn't a dependency that we declare there. Not sure if including it in the composer.json file would solve our sculpin problems or not.

Other than that, everything works as planned! I was using VirtualBox + Vagrant and there was a noticeable lag between saving and gulp noticing, but that's the issue we're addressing using VMWare.

@becw
Copy link
Member

becw commented Mar 4, 2016

Hm, can you require butler by adding a package.json to the project that is using butler? I would expect to do something like this:

npm init
npm install git+ssh://[email protected]:palantirnet/butler.git[#remove-ruby]

@labbydev
Copy link
Contributor Author

labbydev commented Mar 4, 2016

Yes in theory, you can add it to a project by just running the npm install git+ssh://[email protected]:palantirnet/butler.git[#remove-ruby]. I say "in theory" because this hasn't been tested

@becw
Copy link
Member

becw commented Mar 4, 2016

Haha, I tested it, it does not work. Here is the package.json I tried:

{
  "name": "test-add-butler",
  "version": "0.0.0",
  "description": "test requiring butler with npm",
  "dependencies": {
    "@palantirnet/butler": "git+ssh://[email protected]:palantirnet/butler.git[#remove-ruby]"
  }
}

npm throws errors, I think telling me that I'm not using the correct repository name:

npm ERR! Command failed: git clone --template=/Users/white/.npm/_git-remotes/_templates --mirror ssh://[email protected]/palantirnet/butler.git%5B.git /Users/white/.npm/_git-remotes/git-ssh-git-github-com-palantirnet-butler-git-5B-git-remove-ruby--71fd335b
npm ERR! Cloning into bare repository '/Users/white/.npm/_git-remotes/git-ssh-git-github-com-palantirnet-butler-git-5B-git-remove-ruby--71fd335b'...
npm ERR! fatal: remote error: 
npm ERR!    is not a valid repository name

Anyway, once this is working, I would consider including a bare-bones npm package file like package.dist.json in this repository, so that projects could copy this file to your repo and then run npm install.

@becw
Copy link
Member

becw commented Mar 4, 2016

Using https doesn't seem to make any difference:

    "@palantirnet/butler": "git+https://[email protected]:palantirnet/butler.git[#remove-ruby]"

@becw
Copy link
Member

becw commented Mar 4, 2016

Ahh, ok!

With this package.json file in my project:

{
  "name": "@palantirnet/test-add-butler",
  "version": "0.0.0",
  "description": "test requiring butler with npm",
  "license": "UNLICENSED",
  "private": true,
  "dependencies": {
    "butler": "git://github.com/palantirnet/butler.git#remove-ruby"
  }
}

I can run npm install and get butler and all of its dependencies :)

@labbydev
Copy link
Contributor Author

labbydev commented Mar 4, 2016

Message is broken for deploy.

/var/www/drupal8-skeleton.local/node_modules/butler/config/options.js:14
options.message = { 'Updated with Butler - [timestamp]' };
                                                        ^
SyntaxError: Unexpected token }
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/var/www/drupal8-skeleton.local/node_modules/butler/Gulpfile.js:17:15)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)

@becw
Copy link
Member

becw commented Mar 4, 2016

So what I would expect when adding butler to a project is that package.json file above. Then, I would be able to run butler with npm run butler.

It could be npm run gulp -- or at least, that should probably work within the butler repo, because gulp is available at node_modules/bin/gulp within butler. I think this command would expect the Gulpfile.js to be in the project, though, which will not be our situation (the point of separating butler is that the gulpfile no longer belongs to the project).

So probably butler should provide a command called butler, which would be a script that basically ran gulp with the correct parameters I think? Anyway we would provide this to npm using the "bin" key in the butler package.json file:

{
  "name": "@palantirnet/butler",
  "bin": {
    "butler": "./bin/butler"
  }
}

I don't know exactly what this script would be made of, but you would run it from your project like npm run butler.

So that was quite the ramble. Here are a few resources:

@becw
Copy link
Member

becw commented Mar 4, 2016

Also we should probably be using a "scoped" package name like @palantirnet/butler or whatever matches the way we do composer package names. Here are some details that didn't really help my understanding of npm scoping: https://docs.npmjs.com/getting-started/scoped-packages

@@ -1,8 +1,8 @@
Rakefile
*.DS_Store
*.sass-cache/
.DS_Store
Copy link
Contributor

Choose a reason for hiding this comment

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

This belongs in your global .gitignore

@gleroux02
Copy link
Member

@labbydev I've gone ahead and added the --project-dir option so that sculpin does not need to be run in the same directory as the styleguide based on https://palantir.atlassian.net/browse/BUTLER-2

@labbydev labbydev merged commit c49123f into master Mar 27, 2016
@labbydev labbydev deleted the remove-ruby branch March 27, 2016 22:39
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.

5 participants