Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

feat(routes): added default 404 route #33

Merged
merged 2 commits into from
Feb 11, 2020
Merged

Conversation

10xLaCroixDrinker
Copy link
Member

No description provided.

@10xLaCroixDrinker 10xLaCroixDrinker requested a review from a team as a code owner February 10, 2020 23:34
@oneamexbot
Copy link
Contributor

oneamexbot commented Feb 10, 2020

📊 Bundle Size Report

file name size on disk gzip
app.js 112.6KB 31.4KB
runtime.js 15KB 5.3KB
vendors.js 128KB 37.9KB
app~vendors.js 403.8KB 105.7KB
legacy/app.js 119.3KB 33KB
legacy/runtime.js 15KB 5.3KB
legacy/vendors.js 163.1KB 44.9KB
legacy/app~vendors.js 410.2KB 107.4KB

☑️ Preflight Checklist:

All questions must be addressed before this PR can be merged.

  • What is the communication plan for this change?
  • Does any documentation need to be updated or added to account for this change? If so was it done already?
  • What is the motivation for this change?
  • Should these changes also be applied to a maintenance branch or any other long lived branch?
  • How was this change tested?
  • Does this change require cross browser checks? Why or why not?
  • Does this change require a performance test prior to merging? Why or why not?
  • Could this be considered a breaking change? Why or why not?
  • Does the change impact caching?
  • Does the change impact HTTP headers?
  • Does the change have any new infrastructure requirements?
  • Does the change affect other versions of the app?
  • Does the change require additional environment variables?
  • What is the impact to tenants?
  • What is the impact to individual users?
  • What is the change to the size of assets?
  • Should integration tests be added to protect against future regressions on this change?

Generated by 🚫 dangerJS against 41a0a27

JAdshead
JAdshead previously approved these changes Feb 10, 2020
@10xLaCroixDrinker
Copy link
Member Author

  • What is the communication plan for this change?
    Inclusion in release notes
  • Does any documentation need to be updated or added to account for this change? If so was it done already?
    no
  • What is the motivation for this change?
    handle route match misses. previously, going through a bad link would keep you on the same page with an error logged from the router. if the user then refreshed they would see the express 404 page. at that point using the3 back button would not work
  • Should these changes also be applied to a maintenance branch or any other long lived branch?
    no
  • How was this change tested?
    unit tests, integration test, manual validation with sample modules.
  • Does this change require cross browser checks? Why or why not?
    no, just a route addition
  • Does this change require a performance test prior to merging? Why or why not?
  • Could this be considered a breaking change? Why or why not?
    no, it is only additive. users can still define their own 404 handler
  • Does the change impact caching?
    no
  • Does the change impact HTTP headers?
    no
  • Does the change have any new infrastructure requirements?
    no
  • Does the change affect other versions of the app?
    no
  • Does the change require additional environment variables?
    no
  • What is the impact to tenants?
    gracefully handled 404s
  • What is the impact to individual users?
    none
  • What is the change to the size of assets?
    none
  • Should integration tests be added to protect against future regressions on this change?
    added

@10xLaCroixDrinker 10xLaCroixDrinker merged commit d8f1bad into master Feb 11, 2020
@10xLaCroixDrinker 10xLaCroixDrinker deleted the feature/default-404 branch February 11, 2020 16:08
@nellyk nellyk added the enhancement New feature or request label Apr 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants