-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Sage 9.0.0-beta.4? #1919
Sage 9.0.0-beta.4? #1919
Conversation
- Removed unnecessary url-loader targeting .woff files - Replaced file-loader with url-loader (file is fallback for url) - PostCSS plugins are all loaded manually vs postcss.config.js
"rules": { | ||
"no-empty-source": null | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personal opinion, but I like having .eslintrc
and .stylelintrc
as separate files so that neomake and other plugins pick up the config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like those tools should respect reading the configuration from package.json
.. both eslint and stylelint support it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed you tweaked stylelint to allow empty files. I submitted a PR which was merged that would preserve the comments across all framework choices when using |
Already did. 6bbc676#diff-096f5c9caeaa5657ef7ea790ecc34f1b
|
Needs to be bumped to Webpack 3.1.0+ due to webpack-contrib/extract-text-webpack-plugin#548 |
48dd684
to
afbaf8f
Compare
app/helpers.php
Outdated
* @return ContainerContract|mixed | ||
* @SuppressWarnings(PHPMD.StaticAccess) | ||
* @param Container $container | ||
* @return Container|mixed | ||
*/ | ||
function sage($abstract = null, $parameters = [], ContainerContract $container = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@QWp6t Still have a reference to ContainerContract
(removed from use
)! 😃
@@ -25,28 +25,26 @@ | |||
}, | |||
"autoload": { | |||
"psr-4": { | |||
"Roots\\Sage\\": "app/lib/Sage/" | |||
"App\\": "app/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is really fine:
- current app/ files aren't psr-4 compatible
- App namespace may not be reused for real classes since App is specific for current app (but I may be wrong here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PSR-4 doesn't limit the use of namespace in this way. PSR-4 is only for autoloading classes.
If you call a class App\Sage\Cheese
, then it will look for it in app/Sage/Cheese.php
, i.e., PSR-4 autoload works as expected.
thx @LeoColomb for catching this
resources/assets/config.json
Outdated
"{app,resources/views}/**/*.php" | ||
"app/**/*.php", | ||
"config/**/*.php", | ||
"resoruces/controllers/**/*.php", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo! 😱 (resources
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol whoops 😳
config/view.php
Outdated
|
||
'namespaces' => [ | ||
// In your views use something like: @include(WC::some.view.or.partial.here) | ||
'WC' => WP_PLUGIN_DIR.'/woocommerce/templates/', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool, but is this aimed to be included by default? Just asking 😇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, I meant for that to be a commented out example. Thanks for catching it.
Can I use this branch for production? I need this for the Foundation / JS fixes mostly. |
@inorbita yes. no. |
@inorbita Yes, Sage 9 beta 3 is stable for production! |
Is Beta 4 being tagged soon? I assume once #1935 is merged? |
I started off just wanting to update some dependencies, but I wound up getting a little carried away. lol
Gonna work on squashing some bugs now.
To see the new post-create-project routine, run the following: