-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Fix the Bridgetown logger and other test improvements #328
Conversation
Hmmm, so it seems the test suite is stable again locally but not in GH actions .... I'll leave that issue open for now |
Yeah I don't know understand why it keeps crashing in the middle of the test suite. Anyway, thanks so much for digging into this again and I'm glad we figured out the "spooky action at a distance"! 😅 |
@@ -226,7 +226,7 @@ def read_config_file(file) | |||
raise ArgumentError, "Configuration file: (INVALID) #{file}".yellow | |||
end | |||
|
|||
Bridgetown.logger.info "Configuration file:", file | |||
Bridgetown.logger.debug "Configuration file:", file |
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.
So by changing this from info to debug, it means that by default the configuration file line is never output to the user's terminal (they'd have to pass the --verbose flag). I'm not necessarily opposed to this, but it's an obvious change from previous behavior.
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.
My thinking here was that it's not really super important information so I reckon it's ok to bump it down a level. The reason I did it now is because it gets logged out when creating a new site. This is because the webpack
command creates a configuration object that reads the file.
This was the reason I inadvertently turned off logging earlier; I was trying to get this one line to stop being logged out when bridgetown new
is run.
Do you think it's something that we absolutely should be logging out without the verbose flag?
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've found it annoying in some of the work I've done with SSR prototypes as well. Let's do it and just make a mention in our changelog/release notes. 👍
* Welcome to 0.21 "Broughton Beach" * Highlight Render as a hosting provider * Relations for resources (belongs_to, has_many, etc.) (#261) * Relations for resources (belongs_to, has_many, etc) * improve resource type comments * Make relations available to Liquid templates * remove stray p methods * Add test for resource relationships * Support pluralized belongs_to keys * make cop happy * Use model origin id for resource id * Add documentation and Liquid test for resource relations * use newer feed gem * Use list for relation accessor examples * Fix nil bug in relations schema * New `Bridgetown::Component` class with a ViewComponent-ish API (#268) * New Component class with a ViewComponent-ish API * Isolate site in components and improve link helpers * Remove unnecessary special render case for ViewComponent * Fix missing variables * Relocate markdownify to Helpers class * Switch to new Rails-like OutputBuffer, more VC support * Support clean render interop between builtin Component and VCs * Add several common Liquid filters in as Ruby helpers Also add output_safety dependency * Additional html_safe usage * Match Rails' capture escaping logic * Change before_render behavior to match latest VC * Add tests and code comments for Bridgetown::Component * Couple of YARD updates * fix cop offense * Fix bug with url_for helper * Correct failing ERB feature test * Use new SEO and feed helpers for the website * Fix odd cop errors * Convert website navbar to ERB component * fix is-active class name * Use resource variable in default website layout * Update documentation (WIP) on components subsystem Also change all occurences of Javascript to JavaScript * Add lint-html addition * Add documentation regarding ViewComponent * Make linthtml happy * Fix typos * Add link to components docs from the ERB page * Add section on ERB output safety to the docs * End-to-end Ruby front matter, templates, and data files (#285) * Major feature addition for Ruby Front Matter and raw templates Also additional work to normalize Liquid & ERB template APIs * Improve docs * improve error messages, support Ruby data files * Add to_json support for Resources * Improve code quality and test rbfm * Refactor front matter importing and add rbfm to layouts * Revert and use regex capture * Remove logging around rbfm * Lots of Ruby files and rbfm documentation, couple of fixes * Better to_json compat * Fix Liquid error on website build * Fix erb render bug in docs * Update Changelog with 0.21 beta details * Fix dotfiles or multiple extension permalinks (resource engine) (#292) * Fix bug which was swallowing dotfiles and multiple extensions * Revert tag refactoring * Remove sassify/scssify filters, html_safe the obfuscate link * Refactor old TODOs and deprecations * Release 0.21 beta 2 * Bugfix for `previous_resource` method * Ensure resources with no output are still transformed * Remove a bunch of global config accessors on site * Switch `plugins new` command to MiniTest in repo * Release Bridgetown 0.21.0.beta3 * Webiste: use proper metadata title for Index page * Add I18n file reloader to the watcher process * Add prototype page warnings of bad configs * Add note to Resources docs about require prototype page collection key * Add CLI command to update webpack config (#270) * Install required packages in webpack enable postcss tool (#319) * Add config directory and move webpack defaults into it (#316) * Add confirmation for overwriting postcss config in tailwindcss and bt-postcss bundled configurations (#317) * Add Bridgetown version to webpack defaults (#322) * Resolve issue with zombie templates in Pagination/Prototype logic * Add layout method to resource and label method to layout (#324) * Add memoization to cache templates in `Bridgetown::Component` (#326) * Fix the Bridgetown logger and other test improvements (#328) * Release 0.21.0.beta4 * Release v0.21.0 Co-authored-by: Ayush Newatia <[email protected]>
I broke the Bridgetown logger by setting
quiet: true
in the Webpack command, not realising that it would globally disable logging. Sorry, my bad and I should have caught it earlier.In this PR I've un-disabled the logging and made a number of other minor tweaks to the tests. The great news is that it seems the test suite was unstable because logging was being disabled so we can close that issue as well.
I also reverted the
test_new_command
to an earlier state when it was checking the log output.Resolves #327