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 default configuration when using assets #1505

Merged
merged 7 commits into from
May 2, 2021

Conversation

jordisala1991
Copy link
Member

Some changes:

  • Add new option to customize gitignore per branch
  • Add new option: assets on a branch
  • Add basic files to standarize the linting on all repos that have assets
  • Add github workflow

Part of sonata-project/SonataAdminBundle#7049

@jordisala1991
Copy link
Member Author

jordisala1991 commented Apr 30, 2021

Example output for admin-bundle

Dispatch files for: admin-bundle
================================

admin-bundle
------------

Files for 3.x
-------------

Delete /.babelrc.js file!
Delete /.eslintrc.js file!
Delete /.stylelintrc.js file!
Delete /postcss.config.js file!
Delete /.github/workflows/frontend.yaml file!
diff --git a/.gitignore b/.gitignore
index c3c28de3f..c66416c45 100644
--- a/.gitignore
+++ b/.gitignore
@@ -69,4 +69,3 @@ src/Resources/public/vendor/jquery/src

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

Files for master
----------------

diff --git a/.babelrc.js b/.babelrc.js
new file mode 100644
index 000000000..8717622ae
--- /dev/null
+++ b/.babelrc.js
@@ -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'],
+};
diff --git a/.eslintrc.js b/.eslintrc.js
new file mode 100644
index 000000000..0ee6ba2c9
--- /dev/null
+++ b/.eslintrc.js
@@ -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],
+  },
+};
diff --git a/.github/workflows/frontend.yaml b/.github/workflows/frontend.yaml
index d0fbdc3ae..ab054648f 100644
--- a/.github/workflows/frontend.yaml
+++ b/.github/workflows/frontend.yaml
@@ -1,19 +1,32 @@
+# DO NOT EDIT THIS FILE!
+#
+# It's auto-generated by sonata-project/dev-kit package.
+
 name: Frontend

 on:
     push:
         branches:
-            - 3.x
             - master
         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:
diff --git a/.gitignore b/.gitignore
index c3c28de3f..cf852708b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -11,62 +11,4 @@ phpunit.xml
 phpstan.neon
 /.phpunit.result.cache
 /docs/_build/
-
-# 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
-
+yarn-error.log
diff --git a/.stylelintrc.js b/.stylelintrc.js
new file mode 100644
index 000000000..c74f60150
--- /dev/null
+++ b/.stylelintrc.js
@@ -0,0 +1,28 @@
+/*!
+ * 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 = {
+  extends: 'stylelint-config-standard',
+  plugins: ['stylelint-scss', 'stylelint-order'],
+  rules: {
+    'at-rule-no-unknown': null,
+    'scss/at-rule-no-unknown': true,
+    'order/order': [
+      'custom-properties',
+      'declarations',
+    ],
+    'order/properties-alphabetical-order': true,
+  },
+};
diff --git a/postcss.config.js b/postcss.config.js
index 7c712ac20..85545b709 100644
--- a/postcss.config.js
+++ b/postcss.config.js
@@ -7,6 +7,12 @@
  * 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 = {
   plugins: {
     autoprefixer: {},

@jordisala1991
Copy link
Member Author

Few things to resolve:

  • There are tests and php-cs-fixer to fix.
  • Maybe the assets option should be called frontend
  • Maybe the json files should be converted to js, to be able to put de: DO NOT TOUCH this file (json does not have comments)

@OskarStark
Copy link
Member

Please also adjust DispatchBranchesProtectionCommand to make this build required

@jordisala1991 jordisala1991 requested review from OskarStark and a team April 30, 2021 14:22
@jordisala1991
Copy link
Member Author

Related to: sonata-project/SonataAdminBundle#7139

@@ -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

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.

@jordisala1991
Copy link
Member Author

Can we merge this one?

@OskarStark OskarStark merged commit 9ebc7bc into sonata-project:master May 2, 2021
@OskarStark
Copy link
Member

I merged and deployed, can you please keep an eye on the DevKit PRs? Thanks

@jordisala1991 jordisala1991 deleted the feature/assets branch May 2, 2021 18:00
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