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

Core app prepare #243

Merged
merged 6 commits into from
Jul 18, 2019
Merged

Core app prepare #243

merged 6 commits into from
Jul 18, 2019

Conversation

artem-zakharchenko
Copy link
Contributor

@artem-zakharchenko artem-zakharchenko commented Jul 17, 2019

Prepares gavel to be used in the Core app.

Changes

  • Removes json-parse-helpfulerror dependency
  • Removes mochify and sinon dev dependencies
  • Removes test:browser pipeline
  • Adds jju save dependency
  • Adds custom parseJson utility wrapper around jju.parse() to stop relying on fs
  • Removes ES features not supported in the stable spec (i.e. spreading in object literals)
  • Adds test suite that ensures gavel can be bundled in the browser
    • The build is configured to throw when any of NodeJS built-in modules are present in it. Therefore, it's the verification of browser compatibility de facto, cancelling any need of a dedicated test pipeline.
  • Adds building of the gavel library into CJSUMD

GitHub

@@ -0,0 +1,11 @@
const { parse } = require('jju/lib/parse');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only utils.js relies on fs. parse module should be working fine in isolation, getting rid of fs dependency, comparing when importing the module entirely:

const jju = require('jju')

lib/units/validateBody.js Outdated Show resolved Hide resolved
@@ -0,0 +1,11 @@
const { parse } = require('jju/lib/parse');

const parseJson = (json, revivew) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a good idea in to have a wrapper not only because of the fs issue, but also because in the past it happened already that we were switching the JSON parser implementations.

@kuba-kubula
Copy link
Member

bravo

@artem-zakharchenko
Copy link
Contributor Author

CI for PR failed due to the flaky test (#139). Restart should resolve the build status.

@artem-zakharchenko artem-zakharchenko force-pushed the core-app-prepare branch 2 times, most recently from 1417868 to ea5b7bf Compare July 18, 2019 12:12
@artem-zakharchenko artem-zakharchenko merged commit 7832fed into master Jul 18, 2019
@artem-zakharchenko artem-zakharchenko deleted the core-app-prepare branch July 18, 2019 13:12
@ApiaryBot
Copy link
Collaborator

🎉 This PR is included in version 7.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Current gavel.js won't work in Webpack
4 participants