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

Add webpack #149

Merged
merged 3 commits into from Nov 23, 2017
Merged

Add webpack #149

merged 3 commits into from Nov 23, 2017

Conversation

rodfersou
Copy link
Member

@rodfersou rodfersou commented Nov 22, 2017

No description provided.

@rodfersou
Copy link
Member Author

falta alterar o readme com instruções de uso, baseado na descrição do sc.recipe.staticresources

@idgserpro
Copy link
Member

@rodfersou a instância não sobe:

zope.configuration.config.ConfigurationExecutionError: <type 'exceptions.OSError'>: [Errno 2] No such file or directory: '/home/05232887602/git/brasil.gov.temas/src/brasil/gov/temas/jbot'
  in:
  File "/home/05232887602/git/brasil.gov.temas/src/brasil/gov/temas/configure.zcml", line 17.2-20.8
    <browser:jbot
        directory="jbot"
        layer="brasil.gov.portal.interfaces.IBrasilGov"
        />

@@ -22,3 +22,4 @@ parts/
src/collective.cover/
var/
/dist/
src/brasil/gov/temas/themes/brasilgovtemas.js
Copy link
Member

@hvelarde hvelarde Nov 23, 2017

Choose a reason for hiding this comment

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

boa! faltou esse aqui: src/brasil/gov/temas/themes/padrao/brasilgovtemas.css e possivelmente também esse src/brasil/gov/temas/themes/padrao/css/padrao.css.map

Copy link
Member

Choose a reason for hiding this comment

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

@hvelarde será que é uma boa mesmo? Será que não seria melhor documentar. Até o plone versiona os arquivos compilados:

https://github.com/plone/plonetheme.barceloneta/blob/master/plonetheme/barceloneta/theme/less/barceloneta-compiled.css

Copy link
Member

@hvelarde hvelarde Nov 23, 2017

Choose a reason for hiding this comment

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

sim e isso é uma pésima ideia que está fazendo crescer os repositórios de forma absurda: imagina só, eu mudo uma letra em um CSS e ele ao invés de mudar uma linha no controle de versões vai armazenar um arquivo de 50KB completamente diferente e sem possibilidade nem o menor sentido de revisar; eu já falei isso para eles lá.

todo arquivo gerado automaticamente deve ficar fora do controle de versões: traduções, JS e CSS minificado, imagens optimizadas... todo.

deve ser sim documentado que sempre tem que rodar o comando do webpack que gera as coisas antes do release.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hvelarde voce não entendeu o por que disso...

Eu adicionei no ignores o arquivo brasilgovtemas.js por que não precisamos dele, só que o webpack insiste em gerá-lo.

Pesquisei um pouco e não existe uma forma oficial de não gerar esse arquivo, então estou ignorando ele.

Copy link
Member Author

Choose a reason for hiding this comment

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

@idgserpro @hvelarde essa idéia de remover arquivos compilados precisa ser melhor estudada ainda; vamos deixar para um próximo passo.

Copy link
Member Author

Choose a reason for hiding this comment

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

@hvelarde para não ter essa implicação de alterar todo arquivo por estar minificado, podemos gerar o arquivo sem minificar, e deixar que o Plone minifique; Dessa forma não iria ter que guardar o arquivo todo a cada alteração

Copy link
Member

Choose a reason for hiding this comment

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

mas eu já pensei e não tem muita ciência: quem desenvolver, gera os arquivos a cada mudança, isso inclusive pode ser automatizado no buildout; que faz o release tem que ficar ligado que tem que gerar os arquivos antes de fazer o release do mesmo jeito que se faz com as traduções, até poderíamos fazer um plugin para o zest.releaser.

quem usa, não tem nada a fazer, pois o arquivo compilado está incluso no egg.

Copy link
Member Author

Choose a reason for hiding this comment

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

entendido! ja está pronto, obrigado

Copy link
Member

Choose a reason for hiding this comment

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

Isso acontece por que por design webpack é feito para processar arquivos JS, então no mínimo ele deve gerar um arquivo JS.

Mas no futuro podemos precisar de um arquivo de js. Então isso vai deixar de ser um problema, correto?

Copy link
Member Author

Choose a reason for hiding this comment

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

@idgserpro hoje em dia os arquivos JS estão no brasil.gov.portal;

Mas não vejo isso como um problema, somente não usamos.

@hvelarde
Copy link
Member

hvelarde commented Nov 23, 2017

@rodfersou @idgserpro precisa rebase do 2.0.

@@ -0,0 +1,69 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Member

Choose a reason for hiding this comment

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

Qual o sentido do rules.xml estar aqui?

Copy link
Member Author

Choose a reason for hiding this comment

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

Facilitar a edição dos arquivos, na prática trabalhamos somente na pasta webpack, e ele copia os arquivos para src/brasil/gov/temas/themes

Copy link
Member

Choose a reason for hiding this comment

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

Então não seria o caso de ignorar eles também lá na pasta themes/padrao também, como foi feito com o css?

Copy link
Member Author

Choose a reason for hiding this comment

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

é uma opção sim, mas eles são alterados muito menos que os outros arquivos, e não são minificados;

conversamos sobre uma possibilidade aqui, e vou tentar mudar isso mais adiante.

Esse pull request ja está grande demais, gostaria de mesclar ele e continuar em branches menores

@@ -0,0 +1,212 @@
<!DOCTYPE html>
Copy link
Member

Choose a reason for hiding this comment

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

O index.html precisa estar aqui?

Copy link
Member Author

Choose a reason for hiding this comment

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

sim, achamos melhor deixar o index.html e o rules.xml na pasta do Webpack, sendo assim sempre que precisar alterar o index.html ou rules.xml, altera no Webpack e ele é copiado para todos os temas.

Copy link
Member

Choose a reason for hiding this comment

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

Então não seria o caso de ignorar eles também lá na pasta themes/padrao também, como foi feito com o css?

Copy link
Member Author

Choose a reason for hiding this comment

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

é uma opção sim, mas eles são alterados muito menos que os outros arquivos, e não são minificados;

conversamos sobre uma possibilidade aqui, e vou tentar mudar isso mais adiante.

Esse pull request ja está grande demais, gostaria de mesclar ele e continuar em branches menores

<![endif]-->
<meta content="width=device-width, initial-scale=1.0" name="viewport">
<link href="https://fonts.googleapis.com/css?family=Titillium+Web" rel="stylesheet">
<link href="brasilgovtemas.css" rel="stylesheet">
Copy link
Member

Choose a reason for hiding this comment

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

O ideal é ter o css registado no Plone. Já tivemos problema de cache quando fazemos sem registar.

@rodfersou rodfersou force-pushed the add-webpack branch 2 times, most recently from 1cc3efe to 1a1295a Compare November 23, 2017 16:32
@rodfersou rodfersou force-pushed the add-webpack branch 5 times, most recently from 2811e3e to 4dd3ff1 Compare November 23, 2017 18:57
@hvelarde hvelarde merged commit 54bd2c0 into 2.0 Nov 23, 2017
@hvelarde hvelarde deleted the add-webpack branch November 23, 2017 19:33
hvelarde pushed a commit that referenced this pull request Dec 22, 2017
* Add webpack

* Run build-brasilgovtemas after run buildout to automatically generate CSS

* Describe default use case when develop the themes
hvelarde pushed a commit that referenced this pull request Dec 22, 2017
* Add webpack

* Run build-brasilgovtemas after run buildout to automatically generate CSS

* Describe default use case when develop the themes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants