Skip to content

create the npm start script to up and running the app - #14

Merged
woliveiras merged 2 commits into
abc-dev:masterfrom
ldepaiva:master
Jul 7, 2016
Merged

create the npm start script to up and running the app#14
woliveiras merged 2 commits into
abc-dev:masterfrom
ldepaiva:master

Conversation

@ldepaiva

@ldepaiva ldepaiva commented Jul 6, 2016

Copy link
Copy Markdown
Contributor

Olá,

Tentei baixar o projeto para tentar contribuir com alguma coisa, mas, ao rodas os scripts do gulp percebi que muitas imagens estavam faltando, estava ocorrendo um erro de 404 ao tentar carregar o script de smooth-scroll. Quando rodava o gulp as imagens e fonts não estavam indo para o diretório build, como também o diretório vendor.

Arrumei essas questões e aproveitei para criar um npm start onde executará as tasks do gulp.

@woliveiras

Copy link
Copy Markdown
Member

Booa @LucasAntoniassi

Valeu cara. Logo menos fazemos o merge.

@darlanmendonca quer dar uma olhada?

@darlanmendonca

Copy link
Copy Markdown
Contributor

desculpa a ausência, tava curtindo uns dias d férias :D

pelo que vi ta show, somente mudaria onde escrever os scripts, para organizar melhor, mas o merge já pod ser feito sem problemas

scripts declarados dentro de um arquivo sh, e n direto no package.json
cria uma pasta scripts na raiz, e la contera arquivos q serao scripts sh, no kso ficaria scripts/start

no package.json

{
"scripts": {
    "start": "scripts/start",
}

e no arquivo start

#!/bin/bash

set -e

./node_modules/gulp/bin/gulp.js html js scss copy server

colocar o script num arquivo separado ira organizar melhor, scripts maiores
a referencia em path para o bin do gulp, é para permitir dar npm start, e rodar o gulp local, sem precisar do gulp global

vamos dar merge, q eu dou pull request nesta separacao

@ldepaiva

ldepaiva commented Jul 7, 2016

Copy link
Copy Markdown
Contributor Author

Eu criei um outro commit para realizar este seu pedido, porém, quando se cria um arquivo .sh precisa conceder permissão para execução com chmod +x, assim, quando coloquei ele para executar com npm start não funcionou devido a falta de permissão.

Então resolvi fazer um teste e percebi que executando com o gulp é melhor devido ao fato de não precisa de permissão para executar e também este comando procura por ./node_modules/gulp/bin/gulp.js caso você não tenha ele instalado globalmente em sua máquina.

Assim não faz necessário a instalação do gulp global para que funcione.

Aproveitei para criar o diretório scripts e mover o deploy.sh para dentro, e também modifiquei este script para executar npm run build

@woliveiras

Copy link
Copy Markdown
Member

Booooa @LucasAntoniassi

@woliveiras
woliveiras merged commit a151bb8 into abc-dev:master Jul 7, 2016
@darlanmendonca

Copy link
Copy Markdown
Contributor

@LucasAntoniassi yeah, qlquer arquivo de script, precisa de +x como permissao,
e no kso de módulo global, eles atrapalham qdo alguem clona o módulo pla primeira vez e quer rodar. Tipo, qualquer dependencia (mesmo as dev), devem ser instaladas qdo o kra da npm install, uma vez que sao declaradas no package.json. Módulos globais não são descritos no package.json, logo geram confusão.

por isto sugiro que coloquemos o uso do gulp como local e não global, se também concordarem

@darlanmendonca

Copy link
Copy Markdown
Contributor

notei q colocou a permission no package.json, isto n é uma boa prática, por mudar o código (git diff) ao rodar o deploy, ou seja, deploy além d deploy, altera um arquivo no repo

@woliveiras

Copy link
Copy Markdown
Member

Qual seria o melhor esquema pra esse deploy manolo?

@darlanmendonca

Copy link
Copy Markdown
Contributor

criei essa issue pra discutirmos @woliveiras #16 o deploy

@darlanmendonca

Copy link
Copy Markdown
Contributor

tbm criei esta PR pra sugerir scripts pra arquivos ao invés de estarem declarados em lógica no package #15

@ldepaiva

ldepaiva commented Jul 8, 2016

Copy link
Copy Markdown
Contributor Author

Ficou bem melhor mesmo valews :)

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.

3 participants