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.

Sign up for GitHub

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

Jump to bottom

Add default configuration when using assets #1505

Merged
merged 7 commits into from
May 2, 2021
Merged

Add default configuration when using assets #1505

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
300c2ec
Add default configuration when using assets
jordisala1991 Apr 30, 2021
8cc0591
Avoid deprecations with faker
jordisala1991 Apr 30, 2021
860b1cd
Rename assets for frontend and usesAssets for hasFrontend
jordisala1991 Apr 30, 2021
78ee35e
Change json files for js files to add needed headers
jordisala1991 Apr 30, 2021
582d5be
Add missing postcss config
jordisala1991 Apr 30, 2021
6f312aa
Fix missing rename to frontend
jordisala1991 Apr 30, 2021
86588cd
Refactor on remove files
jordisala1991 May 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 59 additions & 60 deletions config/projects.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,74 +1,73 @@
admin-bundle:
phpstan: true
psalm: true

custom_gitignore_part: |
# clean up some non bower ready package
src/Resources/public/vendor/admin-lte/bootstrap
src/Resources/public/vendor/admin-lte/build
src/Resources/public/vendor/admin-lte/dist/img
!src/Resources/public/vendor/admin-lte/dist/img/boxed-bg.jpg # Only image called in AdminLTE.css
src/Resources/public/vendor/admin-lte/dist/js/app.js
src/Resources/public/vendor/admin-lte/dist/js/demo.js
src/Resources/public/vendor/admin-lte/dist/js/pages
src/Resources/public/vendor/admin-lte/documentation
src/Resources/public/vendor/admin-lte/pages
src/Resources/public/vendor/admin-lte/plugins
src/Resources/public/vendor/admin-lte/*.html
src/Resources/public/vendor/admin-lte/Gruntfile.js

src/Resources/public/vendor/iCheck/skins/all.css
src/Resources/public/vendor/iCheck/skins/*/*
!src/Resources/public/vendor/iCheck/skins/square/blue.css
!src/Resources/public/vendor/iCheck/skins/square/blue.png
!src/Resources/public/vendor/iCheck/skins/square/[email protected]
src/Resources/public/vendor/iCheck/icheck.js

src/Resources/public/vendor/slimScroll/examples
src/Resources/public/vendor/slimScroll/jquery.slimscroll.js

src/Resources/public/vendor/bootstrap/grunt
src/Resources/public/vendor/bootstrap/less
src/Resources/public/vendor/bootstrap/test-infra
src/Resources/public/vendor/bootstrap/Gruntfile.js

src/Resources/public/vendor/jqueryui/themes/black-tie
src/Resources/public/vendor/jqueryui/themes/blitzer
src/Resources/public/vendor/jqueryui/themes/cupertino
src/Resources/public/vendor/jqueryui/themes/dark-hive
src/Resources/public/vendor/jqueryui/themes/dot-luv
src/Resources/public/vendor/jqueryui/themes/eggplant
src/Resources/public/vendor/jqueryui/themes/excite-bike
src/Resources/public/vendor/jqueryui/themes/hot-sneaks
src/Resources/public/vendor/jqueryui/themes/humanity
src/Resources/public/vendor/jqueryui/themes/le-frog
src/Resources/public/vendor/jqueryui/themes/mint-choc
src/Resources/public/vendor/jqueryui/themes/overcast
src/Resources/public/vendor/jqueryui/themes/pepper-grinder
src/Resources/public/vendor/jqueryui/themes/redmond
src/Resources/public/vendor/jqueryui/themes/smoothness
src/Resources/public/vendor/jqueryui/themes/south-street
src/Resources/public/vendor/jqueryui/themes/start
src/Resources/public/vendor/jqueryui/themes/sunny
src/Resources/public/vendor/jqueryui/themes/swanky-purse
src/Resources/public/vendor/jqueryui/themes/trontastic
src/Resources/public/vendor/jqueryui/themes/ui-darkness
src/Resources/public/vendor/jqueryui/themes/ui-lightness
src/Resources/public/vendor/jqueryui/themes/vader

src/Resources/public/vendor/jquery/src

src/Resources/public/vendor/jquery.scrollTo/demo
src/Resources/public/vendor/jquery.scrollTo/tests

branches:
master:
php: ['7.3', '7.4', '8.0']
frontend: true
variants:
symfony/symfony: ['4.4', '5.2']
sonata-project/block-bundle: ['4']
3.x:
php: ['7.3', '7.4', '8.0']
custom_gitignore_part: |
# clean up some non bower ready package
src/Resources/public/vendor/admin-lte/bootstrap
src/Resources/public/vendor/admin-lte/build
src/Resources/public/vendor/admin-lte/dist/img
!src/Resources/public/vendor/admin-lte/dist/img/boxed-bg.jpg # Only image called in AdminLTE.css
src/Resources/public/vendor/admin-lte/dist/js/app.js
src/Resources/public/vendor/admin-lte/dist/js/demo.js
src/Resources/public/vendor/admin-lte/dist/js/pages
src/Resources/public/vendor/admin-lte/documentation
src/Resources/public/vendor/admin-lte/pages
src/Resources/public/vendor/admin-lte/plugins
src/Resources/public/vendor/admin-lte/*.html
src/Resources/public/vendor/admin-lte/Gruntfile.js

src/Resources/public/vendor/iCheck/skins/all.css
src/Resources/public/vendor/iCheck/skins/*/*
!src/Resources/public/vendor/iCheck/skins/square/blue.css
!src/Resources/public/vendor/iCheck/skins/square/blue.png
!src/Resources/public/vendor/iCheck/skins/square/[email protected]
src/Resources/public/vendor/iCheck/icheck.js

src/Resources/public/vendor/slimScroll/examples
src/Resources/public/vendor/slimScroll/jquery.slimscroll.js

src/Resources/public/vendor/bootstrap/grunt
src/Resources/public/vendor/bootstrap/less
src/Resources/public/vendor/bootstrap/test-infra
src/Resources/public/vendor/bootstrap/Gruntfile.js

src/Resources/public/vendor/jqueryui/themes/black-tie
src/Resources/public/vendor/jqueryui/themes/blitzer
src/Resources/public/vendor/jqueryui/themes/cupertino
src/Resources/public/vendor/jqueryui/themes/dark-hive
src/Resources/public/vendor/jqueryui/themes/dot-luv
src/Resources/public/vendor/jqueryui/themes/eggplant
src/Resources/public/vendor/jqueryui/themes/excite-bike
src/Resources/public/vendor/jqueryui/themes/hot-sneaks
src/Resources/public/vendor/jqueryui/themes/humanity
src/Resources/public/vendor/jqueryui/themes/le-frog
src/Resources/public/vendor/jqueryui/themes/mint-choc
src/Resources/public/vendor/jqueryui/themes/overcast
src/Resources/public/vendor/jqueryui/themes/pepper-grinder
src/Resources/public/vendor/jqueryui/themes/redmond
src/Resources/public/vendor/jqueryui/themes/smoothness
src/Resources/public/vendor/jqueryui/themes/south-street
src/Resources/public/vendor/jqueryui/themes/start
src/Resources/public/vendor/jqueryui/themes/sunny
src/Resources/public/vendor/jqueryui/themes/swanky-purse
src/Resources/public/vendor/jqueryui/themes/trontastic
src/Resources/public/vendor/jqueryui/themes/ui-darkness
src/Resources/public/vendor/jqueryui/themes/ui-lightness
src/Resources/public/vendor/jqueryui/themes/vader

src/Resources/public/vendor/jquery/src

src/Resources/public/vendor/jquery.scrollTo/demo
src/Resources/public/vendor/jquery.scrollTo/tests
variants:
symfony/symfony: ['4.4']
sonata-project/block-bundle: ['3']
Expand Down
4 changes: 4 additions & 0 deletions src/Command/Dispatcher/DispatchBranchesProtectionCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,10 @@ private function buildRequiredStatusChecks(Project $project, Branch $branch): ar
$requiredStatusChecks[] = 'Psalm';
}

if ($branch->hasFrontend()) {
$requiredStatusChecks[] = 'Webpack Encore';
}

/** @var PhpVersion $phpVersion */
foreach ($branch->phpVersions() as $phpVersion) {
$requiredStatusChecks[] = sprintf(
Expand Down
58 changes: 21 additions & 37 deletions src/Command/Dispatcher/DispatchFilesCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,54 +257,38 @@ private function deleteNotNeededFilesAndDirs(Project $project, Branch $branch, s
));
}

if (!$project->hasDocumentation()) {
$docsPath = $branch->docsPath()->toString();

$docsDirectory = u($distPath)
->append('/')
->append($docsPath)
->toString();

if ($this->filesystem->exists($docsDirectory)) {
$this->io->writeln(sprintf('Delete <info>/%s</info> directory!', $docsPath));
$this->filesystem->remove($docsDirectory);
}

$filepath = '.github/workflows/documentation.yaml';
$documentationWorkflowFile = u($distPath)
->append('/')
->append($filepath)
->toString();
$filesToRemove = [];

if ($this->filesystem->exists($documentationWorkflowFile)) {
$this->io->writeln(sprintf('Delete <info>/%s</info> file!', $filepath));
$this->filesystem->remove($documentationWorkflowFile);
}
if (!$project->hasDocumentation()) {
$filesToRemove[] = $branch->docsPath()->toString();
$filesToRemove[] = '.github/workflows/documentation.yaml';
}

if (!$project->usesPHPStan() && !$project->usesPsalm()) {
$filepath = '.github/workflows/qa.yaml';
$qaWorkflowFile = u($distPath)
->append('/')
->append($filepath)
->toString();
$filesToRemove[] = '.github/workflows/qa.yaml';
}

if ($this->filesystem->exists($qaWorkflowFile)) {
$this->io->writeln(sprintf('Delete <info>/%s</info> file!', $filepath));
$this->filesystem->remove($qaWorkflowFile);
}
if (!$branch->hasFrontend()) {
$filesToRemove[] = '.babelrc.js';
$filesToRemove[] = '.eslintrc.js';
$filesToRemove[] = '.stylelintrc.js';
$filesToRemove[] = 'postcss.config.js';
$filesToRemove[] = '.github/workflows/frontend.yaml';
}

if (!$project->isBundle()) {
$filepath = '.symfony.bundle.yaml';
$symfonyBundleFile = u($distPath)
$filesToRemove[] = '.symfony.bundle.yaml';
}

foreach ($filesToRemove as $fileToRemove) {
$file = u($distPath)
->append('/')
->append($filepath)
->append($fileToRemove)
->toString();

if ($this->filesystem->exists($symfonyBundleFile)) {
$this->io->writeln(sprintf('Delete <info>/%s</info> file!', $filepath));
$this->filesystem->remove($symfonyBundleFile);
if ($this->filesystem->exists($file)) {
$this->io->writeln(sprintf('Delete <info>/%s</info> file!', $fileToRemove));
$this->filesystem->remove($file);
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/Config/ProjectsConfiguration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public function getConfigTreeBuilder(): TreeBuilder
->arrayNode('tools')->prototype('scalar')->defaultValue([])->end()->end()
->arrayNode('php_extensions')->prototype('scalar')->defaultValue([])->end()->end()
->scalarNode('target_php')->defaultNull()->end()
->scalarNode('custom_gitignore_part')->defaultNull()->end()
Copy link
Member

Choose a reason for hiding this comment

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

Some option like panther, phpstan, psalm are on the global level, and some options like php are in the branches level. Some option like custom_gitignore_part are now on both.

It seems weird to have some github related workflow phpstan on the global level but not frontend.
I understand why you did this but we should have the same strategy for every option IMHO.

Should we

  1. Move the all options to the branches level ? It will be more verbose (with duplicate values) but more configurable.
  2. Duplicate all the options on the branches level and the global level ? When it's the same value for every branches (for instance php version) it will avoid duplicates lines.
  3. Wait for the 4.0 release to introduce the frontend option (directly on the global level ?)

Also if I use custom_gitignore_part on the branch level I would have expect to override the project level.
So I would have not expect the code

{% if branch.customGitignorePart is not empty %}
{%- elseif project.customGitignorePart is not empty -%}

But only a check on branch.customGitignorePart with a getter returning project.customGitignorePart by default if the branch level option is not set.

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO we should introduce options as we need them, to avoid implementing things just for having them. Remember this code is not intended to be public used, just internal tool for sonata.

And about what happens if you define at branch level and at root level, I think it should take precedence the branch over the project configuration, I thought I have done that part right, the if will check first for branch config, and then for project config.

Copy link
Member

@VincentLanglet VincentLanglet May 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Oh yes indeed, it's an elseif.

->scalarNode('frontend')->defaultFalse()->end()
->arrayNode('variants')
->normalizeKeys(false)
->useAttributeAsKey('name')
Expand Down
18 changes: 18 additions & 0 deletions src/Domain/Value/Branch.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ final class Branch
*/
private array $variants;

private bool $frontend;
private ?string $customGitignorePart;
private Path $docsPath;
private Path $testsPath;
private PhpVersion $targetPhpVersion;
Expand All @@ -56,6 +58,8 @@ private function __construct(
array $tools,
array $phpExtensions,
array $variants,
?string $customGitignorePart,
bool $frontend,
Path $docsPath,
Path $testsPath,
?PhpVersion $targetPhpVersion
Expand All @@ -65,6 +69,8 @@ private function __construct(
$this->tools = $tools;
$this->phpExtensions = $phpExtensions;
$this->variants = $variants;
$this->customGitignorePart = $customGitignorePart;
$this->frontend = $frontend;
$this->docsPath = $docsPath;
$this->testsPath = $testsPath;
$this->targetPhpVersion = $targetPhpVersion ?? end($this->phpVersions);
Expand Down Expand Up @@ -103,6 +109,8 @@ public static function fromValues(string $name, array $config): self
$tools,
$phpExtensions,
$variants,
$config['custom_gitignore_part'],
$config['frontend'],
Path::fromString($config['docs_path']),
Path::fromString($config['tests_path']),
$targetPhpVersion
Expand Down Expand Up @@ -171,6 +179,16 @@ public function variants(): array
return $this->variants;
}

public function hasFrontend(): bool
{
return $this->frontend;
}

public function customGitignorePart(): ?string
{
return $this->customGitignorePart;
}

public function docsPath(): Path
{
return $this->docsPath;
Expand Down
18 changes: 18 additions & 0 deletions templates/project/.babelrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*!
* This file is part of the Sonata Project package.
*
* (c) Thomas Rabaix <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

/*
* DO NOT EDIT THIS FILE!
*
* It's auto-generated by sonata-project/dev-kit package.
*/

module.exports = {
presets: ['@babel/preset-env'],
};
36 changes: 36 additions & 0 deletions templates/project/.eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*!
* This file is part of the Sonata Project package.
*
* (c) Thomas Rabaix <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

/*
* DO NOT EDIT THIS FILE!
*
* It's auto-generated by sonata-project/dev-kit package.
*/

module.exports = {
parser: '@babel/eslint-parser',
extends: ['airbnb-base'],
env: {
browser: true,
jquery: true,
},
plugins: ['header'],
rules: {
'header/header': [2, 'block', [
'!',
' * This file is part of the Sonata Project package.',
' *',
' * (c) Thomas Rabaix <[email protected]>',
' *',
' * For the full copyright and license information, please view the LICENSE',
' * file that was distributed with this source code.',
' ',
], 2],
},
};
58 changes: 58 additions & 0 deletions templates/project/.github/workflows/frontend.yaml.twig
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# DO NOT EDIT THIS FILE!
#
# It's auto-generated by sonata-project/dev-kit package.

name: Frontend

on:
push:
branches:
{% for branch in project.branchesReverse|filter(branch => branch.hasFrontend) %}
- {{ branch.name }}
{% endfor %}
paths:
- assets/**
- webpack.config.js
- package.json
- yarn.lock
- .babelrc.js
- .eslintrc.js
- .stylelintrc.js
- postcss.config.js
pull_request:
paths:
- assets/**
- webpack.config.js
- package.json
- yarn.lock
- .babelrc.js
- .eslintrc.js
- .stylelintrc.js
- postcss.config.js

jobs:
webpack-encore:
name: Webpack Encore

runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v2

- name: Install Node 14
uses: actions/setup-node@v2
with:
node-version: 14

- name: Install NPM dependencies
uses: bahmutov/npm-install@v1

- name: Run Eslint
run: yarn run eslint assets/js

- name: Run Stylelint
run: yarn run stylelint assets/scss

- name: Run Webpack Encore
run: yarn encore production
Loading